PRs Need Feedback #1063
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
| # This workflow identifies open PRs that need review and updates a tracking issue with the list. | |
| # | |
| # A PR is considered "needing review" when: | |
| # - It is not a draft | |
| # - It does not have the NO-MERGE label | |
| # - It has not been approved (or approval was superseded by changes requested/dismissed) | |
| # - The last activity (comment, review, or commit) was from the PR author or prompter (for bot PRs) | |
| # | |
| # The workflow runs on demand and updates issue #13834 with a table of PRs needing review, | |
| # sorted by waiting time (oldest first). | |
| name: PRs Need Feedback | |
| on: | |
| workflow_dispatch: | |
| schedule: | |
| # Runs every 2 hours | |
| - cron: '0 */2 * * *' | |
| permissions: | |
| issues: write | |
| pull-requests: read | |
| jobs: | |
| get-prs-needing-review: | |
| runs-on: ubuntu-latest | |
| if: ${{ github.repository_owner == 'microsoft' }} | |
| outputs: | |
| prs: ${{ steps.get-prs.outputs.prs }} | |
| steps: | |
| - name: Get PRs needing review | |
| id: get-prs | |
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | |
| with: | |
| script: | | |
| // Set to true to enable debug logging for date calculations | |
| const DEBUG_LOGGING = false; | |
| // Helper function to handle pagination and get all data | |
| async function getAllPages(method, params) { | |
| const results = []; | |
| let page = 1; | |
| while (true) { | |
| const response = await method({ ...params, per_page: 100, page }); | |
| results.push(...response.data); | |
| if (response.data.length < 100) { | |
| break; | |
| } | |
| page++; | |
| } | |
| return results; | |
| } | |
| // Get all open PRs | |
| const pullRequests = await getAllPages(github.rest.pulls.list, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| sort: 'updated', | |
| direction: 'desc' | |
| }); | |
| const prsNeedingReview = []; | |
| for (const pr of pullRequests) { | |
| // Skip draft PRs | |
| if (pr.draft) { | |
| continue; | |
| } | |
| // Skip PRs created by bots | |
| if (pr.user.login.endsWith('[bot]')) { | |
| continue; | |
| } | |
| // Skip PRs with NO-MERGE label | |
| if (pr.labels.some(label => label.name === 'NO-MERGE')) { | |
| continue; | |
| } | |
| // Fetch the full PR details to get mergeable_state (not available in list endpoint) | |
| const { data: fullPr } = await github.rest.pulls.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number | |
| }); | |
| // Check if PR has merge conflicts | |
| // mergeable_state can be: clean, dirty, blocked, behind, unstable, unknown, or null | |
| // - 'dirty' definitely means conflicts | |
| // - mergeable === false + not blocked/behind/unstable also likely means conflicts | |
| // - blocked/behind/unstable mean the PR can't merge due to other reasons (checks, needs update, etc.) | |
| const hasMergeConflict = fullPr.mergeable_state === 'dirty' || | |
| (fullPr.mergeable === false && | |
| fullPr.mergeable_state !== 'blocked' && | |
| fullPr.mergeable_state !== 'behind' && | |
| fullPr.mergeable_state !== 'unstable' && | |
| fullPr.mergeable_state !== 'unknown' && | |
| fullPr.mergeable_state !== 'clean'); | |
| // Get reviews to check if PR is approved | |
| const reviews = await getAllPages(github.rest.pulls.listReviews, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number | |
| }); | |
| // Check if there's a current approval by looking at each reviewer's most recent review | |
| // A reviewer might approve, then later request changes - we need the latest state | |
| const reviewerStates = new Map(); | |
| for (const review of reviews) { | |
| // Only consider meaningful review states (not COMMENTED or PENDING) | |
| if (['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes(review.state)) { | |
| const reviewer = review.user.login.toLowerCase(); | |
| const existingReview = reviewerStates.get(reviewer); | |
| const reviewDate = new Date(review.submitted_at); | |
| if (!existingReview || reviewDate > existingReview.date) { | |
| reviewerStates.set(reviewer, { state: review.state, date: reviewDate }); | |
| } | |
| } | |
| } | |
| // PR is approved if at least one reviewer's latest state is APPROVED | |
| const isApproved = Array.from(reviewerStates.values()).some(r => r.state === 'APPROVED'); | |
| if (isApproved) { | |
| continue; | |
| } | |
| // Get the PR author | |
| const prAuthor = pr.user.login; | |
| // For Copilot-created PRs, check if there's a prompter | |
| // Bot accounts have '[bot]' suffix in their username | |
| const isBotPr = prAuthor.endsWith('[bot]'); | |
| // Get the prompter from the PR body if it's a bot PR | |
| let prompter = null; | |
| if (isBotPr && pr.body) { | |
| // Look for patterns like "Triggered by @username" or "Requested by @username" | |
| const prompterMatch = pr.body.match(/(?:triggered|requested|prompted)\s+by\s+@(\w+)/i); | |
| if (prompterMatch) { | |
| prompter = prompterMatch[1]; | |
| } | |
| } | |
| // Users who can't approve this PR (author and prompter) | |
| const cannotApprove = new Set([prAuthor.toLowerCase()]); | |
| if (prompter) { | |
| cannotApprove.add(prompter.toLowerCase()); | |
| } | |
| // Get issue comments (regular comments) | |
| const comments = await getAllPages(github.rest.issues.listComments, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: pr.number | |
| }); | |
| // Get review comments (inline code comments) | |
| const reviewComments = await getAllPages(github.rest.pulls.listReviewComments, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number | |
| }); | |
| // Get commits to check the last pusher | |
| const commits = await getAllPages(github.rest.pulls.listCommits, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| pull_number: pr.number | |
| }); | |
| // Get timeline events to find when PR was marked ready for review | |
| const timelineEvents = await getAllPages(github.rest.issues.listEventsForTimeline, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: pr.number | |
| }); | |
| // Find the last "ready_for_review" event, or use PR creation date | |
| let readyForReviewDate = new Date(pr.created_at); | |
| for (const event of timelineEvents) { | |
| if (event.event === 'ready_for_review') { | |
| // Use most recent ready for review event date. | |
| readyForReviewDate = new Date(event.created_at); | |
| } | |
| } | |
| if (DEBUG_LOGGING) { | |
| console.log(`\n=== PR #${pr.number}: ${pr.title} ===`); | |
| console.log(`Author: ${prAuthor}, Prompter: ${prompter || 'none'}`); | |
| console.log(`Cannot approve: ${Array.from(cannotApprove).join(', ')}`); | |
| console.log(`Created: ${pr.created_at}, Ready for review: ${readyForReviewDate.toISOString()}`); | |
| console.log(`Timeline events (${timelineEvents.length} total):`); | |
| for (const event of timelineEvents) { | |
| console.log(` ${event.event} at ${event.created_at || 'no date'}`); | |
| } | |
| } | |
| // Collect all internal (author/prompter) and external (reviewers) activities | |
| const internalActivities = []; | |
| const externalActivities = []; | |
| // Helper to check if a user is Copilot or a bot we should ignore | |
| const isCopilotOrBot = (login) => { | |
| const lowerLogin = login.toLowerCase(); | |
| return lowerLogin === 'copilot' || lowerLogin.endsWith('[bot]'); | |
| }; | |
| // Process commits | |
| for (const commit of commits) { | |
| const commitAuthor = commit.author?.login?.toLowerCase(); | |
| const commitCommitter = commit.committer?.login?.toLowerCase(); | |
| const commitDate = new Date(commit.commit.committer.date); | |
| // Skip Copilot activities | |
| if ((commitAuthor && isCopilotOrBot(commitAuthor)) || (commitCommitter && isCopilotOrBot(commitCommitter))) { | |
| if (DEBUG_LOGGING) { | |
| console.log(` Commit ${commit.sha.substring(0, 7)} (${commitDate.toISOString()}): SKIPPED (Copilot)`); | |
| } | |
| continue; | |
| } | |
| const isAuthorInternal = commitAuthor && cannotApprove.has(commitAuthor); | |
| const isCommitterInternal = commitCommitter && cannotApprove.has(commitCommitter); | |
| if (isAuthorInternal || isCommitterInternal) { | |
| internalActivities.push(commitDate); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Commit ${commit.sha.substring(0, 7)} (${commitDate.toISOString()}): INTERNAL (author: ${commitAuthor}, committer: ${commitCommitter})`); | |
| } | |
| } else { | |
| externalActivities.push(commitDate); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Commit ${commit.sha.substring(0, 7)} (${commitDate.toISOString()}): EXTERNAL (author: ${commitAuthor}, committer: ${commitCommitter})`); | |
| } | |
| } | |
| // Commits from users outside cannotApprove are treated as external activities | |
| } | |
| // Process issue comments | |
| for (const comment of comments) { | |
| const commentUser = comment.user.login.toLowerCase(); | |
| // Skip Copilot activities | |
| if (isCopilotOrBot(commentUser)) { | |
| if (DEBUG_LOGGING) { | |
| console.log(` Comment by ${commentUser} (${comment.created_at}): SKIPPED (Copilot)`); | |
| } | |
| continue; | |
| } | |
| const commentDate = new Date(comment.created_at); | |
| if (cannotApprove.has(commentUser)) { | |
| internalActivities.push(commentDate); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Comment by ${commentUser} (${comment.created_at}): INTERNAL`); | |
| } | |
| } else { | |
| externalActivities.push(commentDate); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Comment by ${commentUser} (${comment.created_at}): EXTERNAL`); | |
| } | |
| } | |
| } | |
| // Process review comments | |
| for (const comment of reviewComments) { | |
| const commentUser = comment.user.login.toLowerCase(); | |
| // Skip Copilot activities | |
| if (isCopilotOrBot(commentUser)) { | |
| if (DEBUG_LOGGING) { | |
| console.log(` Review comment by ${commentUser} (${comment.created_at}): SKIPPED (Copilot)`); | |
| } | |
| continue; | |
| } | |
| const commentDate = new Date(comment.created_at); | |
| if (cannotApprove.has(commentUser)) { | |
| internalActivities.push(commentDate); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Review comment by ${commentUser} (${comment.created_at}): INTERNAL`); | |
| } | |
| } else { | |
| externalActivities.push(commentDate); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Review comment by ${commentUser} (${comment.created_at}): EXTERNAL`); | |
| } | |
| } | |
| } | |
| // Process reviews | |
| for (const review of reviews) { | |
| const reviewUser = review.user.login.toLowerCase(); | |
| // Skip Copilot activities | |
| if (isCopilotOrBot(reviewUser)) { | |
| if (DEBUG_LOGGING) { | |
| console.log(` Review by ${reviewUser} (${review.submitted_at}): SKIPPED (Copilot)`); | |
| } | |
| continue; | |
| } | |
| if (!cannotApprove.has(reviewUser) && review.submitted_at) { | |
| externalActivities.push(new Date(review.submitted_at)); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Review by ${reviewUser} (${review.submitted_at}): EXTERNAL (state: ${review.state})`); | |
| } | |
| } else if (DEBUG_LOGGING) { | |
| console.log(` Review by ${reviewUser} (${review.submitted_at}): IGNORED (cannotApprove or no date)`); | |
| } | |
| } | |
| // Find last activity for each group | |
| const lastInternalActivity = internalActivities.length > 0 | |
| ? internalActivities.reduce((a, b) => a > b ? a : b) | |
| : null; | |
| const lastExternalActivity = externalActivities.length > 0 | |
| ? externalActivities.reduce((a, b) => a > b ? a : b) | |
| : null; | |
| // Find the first internal activity after the last external activity | |
| let firstInternalAfterExternal = null; | |
| if (internalActivities.length > 0 && lastExternalActivity) { | |
| const sortedInternalActivities = [...internalActivities].sort((a, b) => a - b); | |
| for (const date of sortedInternalActivities) { | |
| if (date > lastExternalActivity) { | |
| firstInternalAfterExternal = date; | |
| break; | |
| } | |
| } | |
| } | |
| if (DEBUG_LOGGING) { | |
| console.log(`Summary:`); | |
| console.log(` Internal activities: ${internalActivities.length}`); | |
| console.log(` External activities: ${externalActivities.length}`); | |
| console.log(` Last internal: ${lastInternalActivity ? lastInternalActivity.toISOString() : 'none'}`); | |
| console.log(` Last external: ${lastExternalActivity ? lastExternalActivity.toISOString() : 'none'}`); | |
| console.log(` First internal after external: ${firstInternalAfterExternal ? firstInternalAfterExternal.toISOString() : 'none'}`); | |
| } | |
| // Determine if the PR needs review: | |
| // The last activity should be from someone who can't approve (author/prompter) | |
| // meaning the ball is in the reviewers' court | |
| let needsReview = false; | |
| if (lastInternalActivity && lastExternalActivity) { | |
| // Both have activity - needs review if author/prompter acted last | |
| needsReview = lastInternalActivity > lastExternalActivity; | |
| } else if (lastInternalActivity && !lastExternalActivity) { | |
| // Only author/prompter has activity - needs review | |
| needsReview = true; | |
| } | |
| // If only external activity or no activity at all, doesn't need review | |
| if (needsReview) { | |
| // Check if PR has the community-contribution label | |
| const isCommunityContribution = pr.labels.some(label => label.name === 'community-contribution'); | |
| // Determine the effective author - for Copilot PRs, use the first non-Copilot assignee | |
| let effectiveAuthor = pr.user.login; | |
| let effectiveAuthorUrl = pr.user.html_url; | |
| if (pr.user.login.toLowerCase() === 'copilot' || pr.user.login.toLowerCase() === 'copilot[bot]') { | |
| const nonCopilotAssignee = pr.assignees.find(a => { | |
| const assigneeLogin = a.login.toLowerCase(); | |
| return assigneeLogin !== 'copilot' && assigneeLogin !== 'copilot[bot]'; | |
| }); | |
| if (nonCopilotAssignee) { | |
| effectiveAuthor = nonCopilotAssignee.login; | |
| effectiveAuthorUrl = nonCopilotAssignee.html_url; | |
| } | |
| } | |
| const updatedAt = firstInternalAfterExternal ? firstInternalAfterExternal.toISOString() : readyForReviewDate.toISOString(); | |
| if (DEBUG_LOGGING) { | |
| console.log(` Needs review: YES`); | |
| console.log(` Updated at: ${updatedAt} (${firstInternalAfterExternal ? 'first internal after external' : 'ready for review date'})`); | |
| } | |
| prsNeedingReview.push({ | |
| number: pr.number, | |
| title: pr.title, | |
| author: effectiveAuthor, | |
| url: pr.html_url, | |
| authorUrl: effectiveAuthorUrl, | |
| updatedAt: updatedAt, | |
| isCommunityContribution: isCommunityContribution, | |
| hasMergeConflict: hasMergeConflict | |
| }); | |
| } else if (DEBUG_LOGGING) { | |
| console.log(` Needs review: NO`); | |
| } | |
| } | |
| // Sort by updated date ascending (oldest first) | |
| prsNeedingReview.sort((a, b) => new Date(a.updatedAt) - new Date(b.updatedAt)); | |
| console.log('Found ' + prsNeedingReview.length + ' PRs needing review'); | |
| core.setOutput('prs', JSON.stringify(prsNeedingReview)); | |
| update-tracking-issue: | |
| runs-on: ubuntu-latest | |
| needs: get-prs-needing-review | |
| steps: | |
| - name: Update tracking issue | |
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | |
| env: | |
| PRS_JSON: ${{ needs.get-prs-needing-review.outputs.prs }} | |
| with: | |
| script: | | |
| // Issue number to update with the PR list | |
| const TRACKING_ISSUE_NUMBER = 13834; | |
| // Unique marker to identify the automated content | |
| const marker = '<!-- pr-review-needed-list -->'; | |
| // Get PRs from previous job (via environment variable to avoid string interpolation issues) | |
| const prsNeedingReview = JSON.parse(process.env.PRS_JSON); | |
| // Format time since last update | |
| function formatTimeSince(dateStr) { | |
| const date = new Date(dateStr); | |
| const now = new Date(); | |
| const diffMs = now - date; | |
| const diffMins = Math.floor(diffMs / (1000 * 60)); | |
| const diffHours = Math.floor(diffMs / (1000 * 60 * 60)); | |
| const diffDays = Math.floor(diffMs / (1000 * 60 * 60 * 24)); | |
| // Determine circle color based on age | |
| let circle; | |
| if (diffDays > 3) { | |
| circle = '🔴'; | |
| } else if (diffDays > 1) { | |
| circle = '🟡'; | |
| } else { | |
| circle = '🟢'; | |
| } | |
| let timeText; | |
| if (diffDays > 0) { | |
| timeText = diffDays + ' day' + (diffDays === 1 ? '' : 's') + ' ago'; | |
| } else if (diffHours > 0) { | |
| timeText = diffHours + ' hour' + (diffHours === 1 ? '' : 's') + ' ago'; | |
| } else { | |
| timeText = diffMins + ' minute' + (diffMins === 1 ? '' : 's') + ' ago'; | |
| } | |
| return circle + ' ' + timeText; | |
| } | |
| // Function to generate a PR table | |
| function generatePRTable(prs, emptyMessage) { | |
| if (prs.length === 0) { | |
| return '_' + emptyMessage + '_\n\n'; | |
| } | |
| let table = '| PR | Author | Waiting for feedback |\n'; | |
| table += '|---|---|---|\n'; | |
| for (const pr of prs) { | |
| const authorLink = '[' + pr.author + '](' + pr.authorUrl + ')'; | |
| const conflictEmoji = pr.hasMergeConflict ? ' ❌' : ''; | |
| const prLink = '[#' + pr.number + ' - ' + pr.title + '](' + pr.url + ')' + conflictEmoji; | |
| const timeSince = formatTimeSince(pr.updatedAt); | |
| table += '| ' + prLink + ' | ' + authorLink + ' | ' + timeSince + ' |\n'; | |
| } | |
| return table + '\n'; | |
| } | |
| // Build the table | |
| let tableContent = marker + '\n'; | |
| tableContent += '## PRs Needing Review\n\n'; | |
| tableContent += '_Last updated: ' + new Date().toISOString() + '_\n\n'; | |
| // Split PRs into community and team PRs | |
| const communityPRs = prsNeedingReview.filter(pr => pr.isCommunityContribution); | |
| const teamPRs = prsNeedingReview.filter(pr => !pr.isCommunityContribution); | |
| // Team PRs table | |
| tableContent += '### Team PRs\n\n'; | |
| tableContent += generatePRTable(teamPRs, 'No team PRs currently need review.'); | |
| // Community PRs table | |
| tableContent += '### Community Contributions\n\n'; | |
| tableContent += generatePRTable(communityPRs, 'No community PRs currently need review.'); | |
| tableContent += '\n_This list is automatically updated every 2 hours._'; | |
| // If this is a manual workflow_dispatch run, just output the markdown to console | |
| if (context.eventName === 'workflow_dispatch') { | |
| console.log('=== Manual workflow run - Issue body (not updating issue) ==='); | |
| console.log(tableContent); | |
| console.log('=== End of issue body ==='); | |
| console.log('Found ' + prsNeedingReview.length + ' PRs needing review (issue not updated for manual runs)'); | |
| return; | |
| } | |
| // Get the current issue | |
| const { data: issue } = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: TRACKING_ISSUE_NUMBER | |
| }); | |
| // Check if the issue body contains our marker | |
| let newBody; | |
| if (issue.body && issue.body.includes(marker)) { | |
| // Replace the existing content from marker to end | |
| const markerIndex = issue.body.indexOf(marker); | |
| newBody = issue.body.substring(0, markerIndex) + tableContent; | |
| } else { | |
| // Append to the existing body | |
| newBody = (issue.body || '') + '\n\n' + tableContent; | |
| } | |
| // Update the issue | |
| await github.rest.issues.update({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: TRACKING_ISSUE_NUMBER, | |
| body: newBody | |
| }); | |
| console.log('Updated issue #' + TRACKING_ISSUE_NUMBER + ' with ' + prsNeedingReview.length + ' PRs needing review'); |