Repository navigation
fix: create API Review check on fork PRs and pass it when review isn't needed - #247
Open
claude[bot] wants to merge 4 commits into
Open
claude[bot] wants to merge 4 commits into
claude[bot] wants to merge 4 commits into
Conversation
Cation is not installed on contributor forks, so writing the API Review and Deprecation Review checks to the head repo meant they were skipped entirely for fork PRs. Write them to the base repo against the head SHA instead, matching how Semver Label Enforcement already works. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U4dtTVsf8UKZDqhP8BCv1c
georgexu99
approved these changes
Oct 29, 2026
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Only look up Cation's own API Review / Deprecation Review check runs by passing `check_name` and `app_id` to `checks.listForRef`, so a same-named run from another app (e.g. a fork PR's Actions job) can't shadow them and the lookup isn't limited to the first page of check runs. - Log Checks API failures in `addOrUpdateAPIReviewCheck` instead of throwing, so API review approval state is still returned and label automation proceeds. - Isolate each PR in the API review cron so one failure doesn't skip the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U4dtTVsf8UKZDqhP8BCv1c
PRs without an API review label, semver-patch PRs, and PRs with the skip-review label now get a completed/success "Not required" API Review check on their head SHA (created if missing, updated otherwise) instead of no check or a neutral "Outdated" one. This lets the check be made required without blocking PRs that don't need API review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U4dtTVsf8UKZDqhP8BCv1c
claude
Bot
changed the title
The labeled(semver-minor) payload predates the api-review/requested label Cation adds in response, so the check was briefly marked as not required. Include the requested label when updating the check from that handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U4dtTVsf8UKZDqhP8BCv1c
Contributor
Author
There was a problem hiding this comment.
I reviewed the two commits pushed since my last review and found no bugs. They widen the PR's scope: Cation now writes a passing API Review check on every PR's head SHA, and the existing approval came before those commits, so a human should look at the new behavior.
What was reviewed:
markNotRequiredin src/api-review-state.ts: it updates Cation's own run when one exists and creates a new run otherwise. It does nothing when the check lookup failed, so it can't create a duplicate run.- The labeled handler now adds
api-review/requestedto its local copy ofprafter the bot adds the label, so the check goes straight to in-progress instead of passing for a moment first. - The README text and the new specs for the not-required paths.
Extended reasoning...
The change moves the API Review and Deprecation Review checks to the base repo so they work on fork PRs. It filters check lookups by check name and app_id, logs Checks API errors instead of throwing them, and handles each PR separately in the cron loop. The two newest commits go further: every PR, including ones with no API review label, now gets a completed SUCCESS check. That is a repo-wide behavior change, and it is meant to prepare for making the check required. No auth, crypto, or data-exposure code is touched. I deferred because the change is about 560 lines, CODEOWNERS assigns these files to the releases and API working groups, and the only human approval came before the scope grew.
jkleinsc
approved these changes
Oct 29, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by John Kleinschmidt · Slack thread
Cation skipped the API Review (and Deprecation Review) check on fork PRs because it wrote the check to the head repo, where the app isn't installed; it now writes the check to the base repo against the head SHA, the same way Semver Label Enforcement already does, so fork PRs such as electron/electron#55074 get the check. PRs that don't need API review (no API review label, not semver-minor or semver-major, or labeled api-review/skip-review) now get a passing "Not required" API Review check on every head SHA instead of no check at all, so the check can be made required on electron/electron without blocking other PRs, and if a PR later gets api-review/requested the passing run is replaced by a new in-progress one. Errors writing the check are now logged instead of stopping the API review label automation. Making API Review a required check on electron/electron is a separate change that this PR doesn't make.
🤖 Generated with Claude Code
https://claude.ai/code/session_01U4dtTVsf8UKZDqhP8BCv1c