Repository navigation
Never finish the gatekeeper without publishing its check - #56
ilyashatalov wants to merge 1 commit into
Conversation
Publish from the event's head sha, keep going when a read fails, and make the manual dispatch actually do something.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| echo "::warning::could not publish the gatekeeper check on $sha" | ||
| fi | ||
| done | ||
| [ "$posted" -gt 0 ] |
There was a problem hiding this comment.
🔴 Partial publish misses current head
When the event SHA is stale, publish succeeds if only that check posts and the current-head post fails. The current head remains unchecked.
Learn more
publish receives both the event SHA and the API's current head SHA. It returns success after any one post, without distinguishing which SHA accepted the check. A stale event SHA can therefore hide a failed post to the current head. The workflow exits successfully while the required context is absent from the PR's actual head.
Example: A review event names commit A, then the PR moves to commit B before this run reads the API. Posting to A succeeds, but posting to B fails after retries. posted equals one, so the run succeeds while B remains blocked without a gatekeeper check.
Recommended fix: Track every unique target SHA and return success only when every post_check succeeds. At minimum, identify the API SHA as mandatory and fail when its post fails, regardless of success on the event SHA.
Was this helpful? React with 👍 or 👎 to provide feedback.
| concurrency: | ||
| group: gatekeeper-${{ github.event.pull_request.number || github.run_id }} | ||
| cancel-in-progress: true | ||
| group: gatekeeper-${{ github.repository }}-${{ github.event.pull_request.number || github.event.inputs.pr || 'none' }} |
There was a problem hiding this comment.
🟡 Reusable calls cancel other PR checks
Without an event-level pr, the concurrency key ignores inputs.pr and groups every target as none. GitHub replaces older pending jobs, leaving some PRs unchecked.
Learn more
The reusable workflow accepts inputs.pr specifically for callers without pull-request event context. Such callers need not expose a same-named github.event.inputs.pr; scheduled or repository-dispatch callers expose none. GitHub permits only one pending run per concurrency group and replaces an older pending run even when cancel-in-progress is false.
Example: A repository-dispatch caller invokes the workflow for PR 12 and then PR 34. Both keys end in none. If PR 12 is still pending, the PR 34 call cancels it before it publishes a check.
Recommended fix: Build the key from github.event.pull_request.number || inputs.pr || 'none'. The unified inputs context also works for direct workflow_dispatch runs.
| group: gatekeeper-${{ github.repository }}-${{ github.event.pull_request.number || github.event.inputs.pr || 'none' }} | |
| group: gatekeeper-${{ github.repository }}-${{ github.event.pull_request.number || inputs.pr || 'none' }} |
Was this helpful? React with 👍 or 👎 to provide feedback.
| # when it matches a built-in docs pattern or a caller-supplied | ||
| # `exempt_paths` regex; the PR is exempt only when every | ||
| # changed file is. | ||
| if ! pr_files="$(gh_retry gh api "repos/$REPO/pulls/$pr/files" --paginate)"; then |
There was a problem hiding this comment.
🟡 Paginated files lose path exemptions
For multiple file pages, pr_files makes exempt emit one result per page. Even all-exempt pages produce multiple true values, disabling the exemption.
Learn more
gh api --paginate writes each REST page as a separate JSON value. The following jq program runs independently on every page, so command substitution captures several newline-separated booleans. The equality test accepts only the exact single value true.
Example: A docs-only PR spans two pages. Each page evaluates to true, making exempt equal true\ntrue; the branch for exempt changes is skipped and the PR receives the normal two-approval verdict.
Recommended fix: Slurp the paginated responses into one value and flatten the page arrays before testing filenames. Use that same flattened array for the displayed file list.
Was this helpful? React with 👍 or 👎 to provide feedback.
| or any($extra[]; . as $p | $f | test($p)) | ||
| )) | tostring | ||
| end | ||
| ' | ||
| ) | ||
|
|
||
| if [ "$EXEMPT" = "true" ]; then | ||
| FILES_LIST=$(echo "$PR_FILES" | jq -r '[.[].filename] | map("- " + .) | join("\n")') | ||
| echo "exempt=true" >> "$GITHUB_OUTPUT" | ||
| echo "conclusion=success" >> "$GITHUB_OUTPUT" | ||
| printf 'summary=%s\n' "Exempt changes detected (docs or caller-exempted paths). Additional approvals are not required." >> "$GITHUB_OUTPUT" | ||
| { | ||
| echo "details<<EOF" | ||
| echo "Exempt files:" | ||
| echo "$FILES_LIST" | ||
| echo "EOF" | ||
| } >> "$GITHUB_OUTPUT" | ||
| echo "approvals_list=" >> "$GITHUB_OUTPUT" | ||
| echo "approvals_missing=0" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| echo "exempt=false" >> "$GITHUB_OUTPUT" | ||
| ')" |
There was a problem hiding this comment.
🔍 Invalid patterns degrade silently
An invalid exempt_paths regex makes jq fail, but execution continues with an empty exempt. The check becomes a normal approval verdict without explaining the configuration error.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| is_bot="$(printf '%s' "$pr_data" | jq -r ' | ||
| ((.author.login // "") | endswith("[bot]")) | ||
| or ((.author.login // "") == "app/devin-ai-integration") | ||
| or ((.author.login // "") == "dl-bot-integrations") | ||
| or (.author.is_bot == true) | ||
| ' | ||
| ) | ||
|
|
||
| if [ "$IS_BOT_AUTHOR" != "true" ]; then | ||
| echo "conclusion=success" >> "$GITHUB_OUTPUT" | ||
| printf 'summary=%s\n' "Not a bot-authored PR. Skipping human-review enforcement." >> "$GITHUB_OUTPUT" | ||
| echo "details=" >> "$GITHUB_OUTPUT" | ||
| echo "is_bot=false" >> "$GITHUB_OUTPUT" | ||
| echo "exempt=false" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| echo "is_bot=true" >> "$GITHUB_OUTPUT" | ||
|
|
||
| # 2b. Exempt changes skip additional approvals. A file is exempt when it | ||
| # matches a built-in docs pattern or a caller-supplied `exempt_paths` | ||
| # regex. A PR is exempt only when *every* changed file is exempt. | ||
| PR_FILES=$(gh api "repos/$REPO/pulls/$PR/files" --paginate) | ||
| EXTRA_PATTERNS=$(printf '%s' "${EXEMPT_PATHS:-}" | jq -R -s 'split("\n") | map(select(length > 0))') | ||
| EXEMPT=$( | ||
| echo "$PR_FILES" | jq --argjson extra "$EXTRA_PATTERNS" -r ' | ||
| ' 2>/dev/null)" |
There was a problem hiding this comment.
🟥 Unknown authors bypass bot review
When GitHub returns a null author, is_bot becomes false and the PR receives a successful check without approvals.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Rewriting the gatekeeper around one rule: it must never finish without publishing the required check, since a missing required context looks exactly like a workflow that never ran, and the PR then just sits there blocked. It now publishes from the event's head sha as well as the API one (a race with a force-push used to send the check to a commit that was no longer in the PR), publishes a red check instead of skipping the publish step when a read fails, stops cancelling itself when review events arrive in bursts, and the manual dispatch takes a PR number and actually does something now.
Two behaviour changes worth flagging: a comment-only review no longer wipes out that person's earlier approval, and a payload it cannot read is a red check rather than a green one. There is deliberately no scheduled sweep, and the README says why: this is the only required status check in crosschain and lazer, so a sweep would green-light head commits that a
GITHUB_TOKENpush left with no build and no test either.