Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
105 changes: 59 additions & 46 deletions .github/workflows/label-pr-review-state.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,76 +6,89 @@ on:
workflow_dispatch:

permissions:
pull-requests: read
issues: write
pull-requests: write

concurrency:
group: label-pr-review-state
cancel-in-progress: false

jobs:
reconcile:
runs-on: ubuntu-latest
steps:
- name: Reconcile PR review state labels
uses: actions/github-script@v7
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
with:
script: |
const { owner, repo } = context.repo;
const stateLabels = ['awaiting-author', 'awaiting-review'];
const failures = [];

const prs = await github.paginate(github.rest.pulls.list, {
owner, repo, state: 'open', per_page: 100,
});

for (const pr of prs) {
const reviews = await github.paginate(github.rest.pulls.listReviews, {
owner, repo, pull_number: pr.number, per_page: 100,
});
try {
const reviews = await github.paginate(github.rest.pulls.listReviews, {
owner, repo, pull_number: pr.number, per_page: 100,
});

// Reviews are returned chronologically, so later entries replace
// each reviewer's earlier decision.
const latest = new Map();
for (const r of reviews) {
if (r.state !== 'COMMENTED') {
latest.set(r.user.login, r);
// Reviews are returned chronologically, so later entries replace
// each reviewer's earlier decision.
const latest = new Map();
for (const r of reviews) {
if (r.state !== 'COMMENTED') {
latest.set(r.user.login, r);
}
Comment on lines +40 to +43

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against deleted reviewer accounts.

If a reviewer's account has been deleted, r.user may be null, causing a null-reference error on r.user.login. This would fail the reconciliation for that PR.

🛡️ Proposed fix to handle null user
                 for (const r of reviews) {
-                  if (r.state !== 'COMMENTED') {
+                  if (r.state !== 'COMMENTED' && r.user) {
                     latest.set(r.user.login, r);
                   }
                 }
📝 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.

Suggested change
for (const r of reviews) {
if (r.state !== 'COMMENTED') {
latest.set(r.user.login, r);
}
for (const r of reviews) {
if (r.state !== 'COMMENTED' && r.user) {
latest.set(r.user.login, r);
}
}
🤖 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/label-pr-review-state.yml around lines 40 - 43, The code
accesses r.user.login without first checking if r.user is null, which will cause
a null-reference error when a reviewer's account has been deleted. Add a guard
condition to check that r.user is not null before attempting to access
r.user.login in the latest.set() call. This should be done in the loop where
reviews are being iterated and the latest map is being populated to ensure null
users are skipped gracefully.

}
}

const changeRequestReviewers = [...latest.entries()]
.filter(([, review]) => review.state === 'CHANGES_REQUESTED')
.map(([login]) => login);
const requestedReviewers = new Set(
pr.requested_reviewers.map(reviewer => reviewer.login),
);
const changeRequestReviewers = [...latest.entries()]
.filter(([, review]) => review.state === 'CHANGES_REQUESTED')
.map(([login]) => login);
const requestedReviewers = new Set(
pr.requested_reviewers.map(reviewer => reviewer.login),
);
Comment on lines +49 to +51

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against undefined requested_reviewers.

The pr.requested_reviewers field might be undefined in some edge cases, causing a crash on .map(...).

🛡️ Proposed fix to handle undefined requested_reviewers
                 const requestedReviewers = new Set(
-                  pr.requested_reviewers.map(reviewer => reviewer.login),
+                  (pr.requested_reviewers || []).map(reviewer => reviewer.login),
                 );
🤖 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/label-pr-review-state.yml around lines 49 - 51, The code
attempts to call .map() on pr.requested_reviewers without checking if it exists
first, which will crash if requested_reviewers is undefined. Guard against this
by adding a null/undefined check before the .map() call in the
requestedReviewers Set initialization, providing an empty array as a fallback
when pr.requested_reviewers is not defined.


let desiredLabel = null;
if (changeRequestReviewers.length > 0) {
desiredLabel = changeRequestReviewers.every(
reviewer => requestedReviewers.has(reviewer),
)
? 'awaiting-review'
: 'awaiting-author';
}
let desiredLabel = null;
if (changeRequestReviewers.length > 0) {
desiredLabel = changeRequestReviewers.every(
reviewer => requestedReviewers.has(reviewer),
)
? 'awaiting-review'
: 'awaiting-author';
}
Comment on lines +46 to +60

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if any open PRs have requested_teams to understand the impact

gh pr list --state open --json number,requestedReviewers,reviews --limit 100 | \
  jq '.[] | select(.requestedReviewers != null) | select(.requestedReviewers | length > 0) | {number, requestedReviewers}'

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 745


🏁 Script executed:

cat -n .github/workflows/label-pr-review-state.yml

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 4121


🌐 Web query:

GitHub API PR requested_teams field structure how to access team review requests

💡 Result:

To access team review requests for a GitHub Pull Request via the REST API, you should use the dedicated review requests endpoint rather than relying solely on the primary Pull Request object [1][2]. Accessing Team Review Requests: The primary Pull Request object (returned by GET /repos/{owner}/{repo}/pulls/{pull_number}) does not include a detailed list of requested teams in its response body. To retrieve this information, you must call the "Get all requested reviewers for a pull request" endpoint [2][3]: GET /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers This endpoint returns a JSON object containing two distinct arrays [2][4]: 1. users: A list of individual users whose review has been requested [2][4]. 2. teams: A list of teams whose review has been requested [2][4]. The team objects within the teams array follow the standard Team structure, which includes the following key fields: - id: The unique identifier for the team [1][2]. - slug: The URL-friendly name of the team [1][2]. - name: The display name of the team [1][2]. - url/html_url: Links to the team's resource [1][2]. - description: A description of the team [1][2]. Library Implementations: If you are using common GitHub API client libraries, they often provide helper methods to abstract this call: - PyGithub: Use the pull_request.get_review_requests method, which returns a tuple containing two PaginatedLists: one for users and one for teams [5][6]. - github3.py: Access the requested_teams attribute on the PullRequest object (which may trigger a call to the review requests endpoint) [7][8]. - Octokit.net: The PullRequest model typically includes a RequestedTeams property that maps the API response to a collection of Team objects [9]. When requesting new reviews, use the corresponding POST endpoint [2][3]: POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers Provide the team_reviewers parameter as an array of team slugs to request reviews from specific teams [2][3].

Citations:


Team review requests are not handled.

The logic only checks pr.requested_reviewers (individual users) from the main PR object, but team review requests require a separate API call. GitHub's /requested_reviewers endpoint returns both users and teams, but the workflow doesn't call it. If a team is requested to review and one of its members submits a "changes requested" review, the workflow can't match the team to update the label correctly.

To fix this, call the additional requested_reviewers endpoint to retrieve both users and teams:

📋 Proposed fix to include team reviewers
                 const reviews = await github.paginate(github.rest.pulls.listReviews, {
                   owner, repo, pull_number: pr.number, per_page: 100,
                 });
 
+                const reviewRequests = await github.rest.pulls.requestedReviewers({
+                  owner, repo, pull_number: pr.number,
+                });
+
                 // Reviews are returned chronologically, so later entries replace
                 // each reviewer's earlier decision.
                 const latest = new Map();
                 for (const r of reviews) {
                   if (r.state !== 'COMMENTED') {
                     latest.set(r.user.login, r);
                   }
                 }
 
                 const changeRequestReviewers = [...latest.entries()]
                   .filter(([, review]) => review.state === 'CHANGES_REQUESTED')
                   .map(([login]) => login);
                 const requestedReviewers = new Set(
-                  pr.requested_reviewers.map(reviewer => reviewer.login),
+                  [
+                    ...reviewRequests.users.map(u => u.login),
+                    ...reviewRequests.teams.map(t => t.slug),
+                  ],
                 );

This requires additional logic to handle the fact that team members submit reviews individually but team requests are at the team level—consider mapping individual team member logins to their team slugs or adding team members as re-requestable reviewers directly.

🤖 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/label-pr-review-state.yml around lines 46 - 60, The logic
for determining the PR label state only considers individual requested reviewers
from pr.requested_reviewers but does not account for team review requests, which
are retrieved via a separate GitHub API endpoint. When a team is requested to
review and one of its members submits a changes-requested review, the workflow
cannot match the team member to the team request, causing incorrect label
assignment. Add a separate API call to fetch team review requests (teams are
returned separately from individual users), then expand the requestedReviewers
set to include both individual user logins and the logins of all members within
requested teams. This ensures that when changeRequestReviewers are checked
against requestedReviewers using the every() method, team members are properly
recognized as part of a requested team review.


const currentLabels = new Set(pr.labels.map(label => label.name));
for (const label of stateLabels) {
if (label !== desiredLabel && currentLabels.has(label)) {
await github.rest.issues.removeLabel({
owner, repo, issue_number: pr.number, name: label,
const currentLabels = new Set(pr.labels.map(label => label.name));
for (const label of stateLabels) {
if (label !== desiredLabel && currentLabels.has(label)) {
await github.rest.issues.removeLabel({
owner, repo, issue_number: pr.number, name: label,
});
}
}

if (desiredLabel && !currentLabels.has(desiredLabel)) {
await github.rest.issues.addLabels({
owner, repo, issue_number: pr.number, labels: [desiredLabel],
});
}
}

if (desiredLabel && !currentLabels.has(desiredLabel)) {
await github.rest.issues.addLabels({
owner, repo, issue_number: pr.number, labels: [desiredLabel],
});
if (
desiredLabel !== 'awaiting-author' &&
currentLabels.has('stale-awaiting-author')
) {
await github.rest.issues.removeLabel({
owner, repo, issue_number: pr.number,
name: 'stale-awaiting-author',
});
}
} catch (error) {
failures.push(`#${pr.number}: ${error.message}`);
core.error(`Failed to reconcile PR #${pr.number}: ${error.message}`);
}
}

if (
desiredLabel !== 'awaiting-author' &&
currentLabels.has('stale-awaiting-author')
) {
await github.rest.issues.removeLabel({
owner, repo, issue_number: pr.number,
name: 'stale-awaiting-author',
});
}
if (failures.length > 0) {
core.setFailed(`Failed to reconcile ${failures.length} PR(s): ${failures.join('; ')}`);
}
4 changes: 2 additions & 2 deletions .github/workflows/stale.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
name: 60-day inactivity
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v9
- uses: actions/stale@5bef64f19d7facfb25b37b414482c7164d639639 # v9
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}

Expand All @@ -38,7 +38,7 @@ jobs:
name: 14-day author inactivity after requested changes
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v9
- uses: actions/stale@5bef64f19d7facfb25b37b414482c7164d639639 # v9
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}

Expand Down
Loading