From c006d81e2c1efbd0b68510e8164b18cea54ff879 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 10:08:52 -0700 Subject: [PATCH 01/12] feat(workflow): update reviewer assignment workflow and codeowners team definitions --- .github/CODEOWNERS | 2 +- .github/workflows/assign-reviewers.yml | 112 +++++++++++++++++-------- 2 files changed, 78 insertions(+), 36 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 519e8cc1a5ad..663b4e2a2feb 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -6,7 +6,7 @@ # The cloud sdk nodejs team is the default owner for nodejs repositories. -# team_members: @pearigee @feywind @danieljbruce @shivanee-p +# team_members: @westarle @feywind @danieljbruce @shivanee-p * @googleapis/cloud-sdk-nodejs-team /handwritten/bigquery @googleapis/bigquery-team /handwritten/cloud-profiler @googleapis/cloud-profiler-team diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index a917a32d7bb3..d9d4a6dff9a1 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -29,7 +29,10 @@ jobs: if (author === 'release-please[bot]' || author === 'release-please') { console.log(`PR opened by ${author}. Ensuring cloud-sdk-nodejs-team is assigned as a reviewer.`); const requestedTeams = context.payload.pull_request.requested_teams || []; - const hasDefaultTeam = requestedTeams.some(team => team.slug === 'cloud-sdk-nodejs-team'); + const hasDefaultTeam = requestedTeams.some(team => + (team.slug && team.slug.toLowerCase() === 'cloud-sdk-nodejs-team') || + (team.name && team.name.toLowerCase() === 'cloud-sdk-nodejs-team') + ); if (!hasDefaultTeam) { try { await github.rest.pulls.requestReviewers({ @@ -48,8 +51,14 @@ jobs: const requestedReviewers = context.payload.pull_request.requested_reviewers || []; const requestedTeams = context.payload.pull_request.requested_teams || []; - const hasDefaultTeam = requestedTeams.some(team => team.slug === 'cloud-sdk-nodejs-team'); - const hasRouteSpecificTeam = requestedTeams.some(team => team.slug !== 'cloud-sdk-nodejs-team'); + const hasDefaultTeam = requestedTeams.some(team => + (team.slug && team.slug.toLowerCase() === 'cloud-sdk-nodejs-team') || + (team.name && team.name.toLowerCase() === 'cloud-sdk-nodejs-team') + ); + const hasRouteSpecificTeam = requestedTeams.some(team => + (team.slug && team.slug.toLowerCase() !== 'cloud-sdk-nodejs-team') && + (!team.name || team.name.toLowerCase() !== 'cloud-sdk-nodejs-team') + ); // Check if PR has already been reviewed let hasReviews = false; @@ -72,7 +81,7 @@ jobs: console.log(`PR already has requested reviewers: ${requestedReviewers.map(r => r.login).join(', ')}.`); shouldExit = true; } else if (hasRouteSpecificTeam) { - console.log(`PR already has route-specific team reviewer requested: ${requestedTeams.map(t => t.slug).join(', ')}.`); + console.log(`PR already has route-specific team reviewer requested: ${requestedTeams.map(t => t.slug || t.name).join(', ')}.`); shouldExit = true; } else if (hasReviews) { shouldExit = true; @@ -99,11 +108,18 @@ jobs: // 2. Get list of files modified in the PR - const { data: files } = await github.rest.pulls.listFiles({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - }); + let files = []; + try { + const res = await github.rest.pulls.listFiles({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + per_page: 100, + }); + files = res.data; + } catch (err) { + console.error("Failed to list PR files:", err.message || err); + } const PATH_ROUTING = [ { prefix: 'handwritten/bigquery/', team: 'bigquery-team' }, @@ -142,14 +158,15 @@ jobs: } if (!assigned) { - console.log("Fetching .github/CODEOWNERS to identify default reviewers..."); + const baseRef = context.payload.pull_request.base.ref || context.payload.pull_request.base.sha || 'main'; + console.log(`Fetching .github/CODEOWNERS (ref: ${baseRef}) to identify default reviewers...`); let defaultReviewers = []; try { const { data: codeownersData } = await github.rest.repos.getContent({ owner: context.repo.owner, repo: context.repo.repo, path: '.github/CODEOWNERS', - ref: context.payload.pull_request.head.sha, + ref: baseRef, }); const content = Buffer.from(codeownersData.content, 'base64').toString('utf-8'); const lines = content.split('\n'); @@ -163,17 +180,27 @@ jobs: defaultReviewers = parts .filter(p => p.startsWith('@')) .map(p => p.substring(1)) - .filter(login => login !== author); + .filter(login => login.toLowerCase() !== author.toLowerCase()); break; } } } } catch (err) { - console.error("Failed to read or parse CODEOWNERS:", err); + console.error("Failed to read or parse CODEOWNERS:", err.message || err); + } + + // Hardcoded fallback list if CODEOWNERS could not be fetched or contained no reviewers (excluding author) + if (defaultReviewers.length === 0) { + console.warn("No default reviewers found in CODEOWNERS. Using fallback team members."); + const FALLBACK_TEAM = ['pearigee', 'feywind', 'danieljbruce', 'shivanee-p']; + defaultReviewers = FALLBACK_TEAM.filter(login => login.toLowerCase() !== author.toLowerCase()); } if (defaultReviewers.length > 0) { console.log(`Found default reviewers: ${defaultReviewers.join(', ')}. Load balancing among them.`); + let candidateLogins = defaultReviewers; + let minLoad = 0; + try { // Retrieve active open PRs to calculate load const { data: openPRs } = await github.rest.pulls.list({ @@ -185,23 +212,25 @@ jobs: const loadMap = {}; for (const member of defaultReviewers) { - loadMap[member] = 0; + loadMap[member.toLowerCase()] = 0; } for (const pr of openPRs) { // Count pending review requests if (pr.requested_reviewers) { for (const reviewer of pr.requested_reviewers) { - if (loadMap[reviewer.login] !== undefined) { - loadMap[reviewer.login]++; + const lower = (reviewer.login || '').toLowerCase(); + if (loadMap[lower] !== undefined) { + loadMap[lower]++; } } } // Count assignees if (pr.assignees) { for (const assignee of pr.assignees) { - if (loadMap[assignee.login] !== undefined) { - loadMap[assignee.login]++; + const lower = (assignee.login || '').toLowerCase(); + if (loadMap[lower] !== undefined) { + loadMap[lower]++; } } } @@ -210,33 +239,45 @@ jobs: console.log("Current team workload:", loadMap); // Find members with the minimum load - let minLoad = Infinity; - let selectedReviewers = []; + minLoad = Infinity; + candidateLogins = []; for (const member of defaultReviewers) { - const load = loadMap[member]; + const load = loadMap[member.toLowerCase()]; if (load < minLoad) { minLoad = load; - selectedReviewers = [member]; + candidateLogins = [member]; } else if (load === minLoad) { - selectedReviewers.push(member); + candidateLogins.push(member); } } + } catch (err) { + console.error("Failed to calculate workload from open PRs, proceeding with all default reviewers:", err.message || err); + } - const leastLoadedReviewer = selectedReviewers[Math.floor(Math.random() * selectedReviewers.length)]; - console.log(`Selected reviewer with least load (load: ${minLoad}): ${leastLoadedReviewer}`); + // Shuffle candidates and attempt to request review from least loaded candidates first, then any default reviewer + const candidateQueue = [ + ...candidateLogins.sort(() => Math.random() - 0.5), + ...defaultReviewers.filter(m => !candidateLogins.includes(m)).sort(() => Math.random() - 0.5) + ]; - await github.rest.pulls.requestReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - reviewers: [leastLoadedReviewer], - }); - assigned = true; - } catch (err) { - console.error("Failed to assign reviewers using load balancing:", err); + for (const candidate of candidateQueue) { + console.log(`Attempting to assign reviewer: ${candidate}`); + try { + await github.rest.pulls.requestReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + reviewers: [candidate], + }); + assigned = true; + console.log(`Successfully assigned reviewer: ${candidate}`); + break; + } catch (reqErr) { + console.error(`Failed to assign reviewer ${candidate}:`, reqErr.message || reqErr); + } } } else { - console.warn("No default reviewers found in CODEOWNERS (or they only contained teams/author)."); + console.warn("No default reviewers available to assign."); } } @@ -254,3 +295,4 @@ jobs: console.warn("Failed to remove default cloud-sdk-nodejs-team reviewer:", err.message || err); } } + From f1d0187950b15e4bfce1bb325881b2c2e66a9098 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 10:10:02 -0700 Subject: [PATCH 02/12] chore(workflow): make assign-reviewers workflow hermetic with local checkout --- .github/workflows/assign-reviewers.yml | 49 ++++++++++++++------------ 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index d9d4a6dff9a1..9240575acf07 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -16,12 +16,18 @@ jobs: runs-on: ubuntu-latest if: github.event.pull_request.draft == false steps: + - name: Checkout repository + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + persist-credentials: false + - name: Assign Reviewers continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | + const fs = require('fs'); const author = context.payload.pull_request.user.login; // Release PRs should remain the responsibility of the oncall, @@ -158,35 +164,32 @@ jobs: } if (!assigned) { - const baseRef = context.payload.pull_request.base.ref || context.payload.pull_request.base.sha || 'main'; - console.log(`Fetching .github/CODEOWNERS (ref: ${baseRef}) to identify default reviewers...`); + console.log("Reading .github/CODEOWNERS locally from repository..."); let defaultReviewers = []; try { - const { data: codeownersData } = await github.rest.repos.getContent({ - owner: context.repo.owner, - repo: context.repo.repo, - path: '.github/CODEOWNERS', - ref: baseRef, - }); - const content = Buffer.from(codeownersData.content, 'base64').toString('utf-8'); - const lines = content.split('\n'); - for (const line of lines) { - const trimmed = line.trim(); - if (trimmed.startsWith('#')) { - const commentContent = trimmed.substring(1).trim(); - if (commentContent.startsWith('team_members:')) { - const listPart = commentContent.substring('team_members:'.length).trim(); - const parts = listPart.split(/\s+/); - defaultReviewers = parts - .filter(p => p.startsWith('@')) - .map(p => p.substring(1)) - .filter(login => login.toLowerCase() !== author.toLowerCase()); - break; + if (fs.existsSync('.github/CODEOWNERS')) { + const content = fs.readFileSync('.github/CODEOWNERS', 'utf-8'); + const lines = content.split('\n'); + for (const line of lines) { + const trimmed = line.trim(); + if (trimmed.startsWith('#')) { + const commentContent = trimmed.substring(1).trim(); + if (commentContent.startsWith('team_members:')) { + const listPart = commentContent.substring('team_members:'.length).trim(); + const parts = listPart.split(/\s+/); + defaultReviewers = parts + .filter(p => p.startsWith('@')) + .map(p => p.substring(1)) + .filter(login => login.toLowerCase() !== author.toLowerCase()); + break; + } } } + } else { + console.warn(".github/CODEOWNERS file not found locally."); } } catch (err) { - console.error("Failed to read or parse CODEOWNERS:", err.message || err); + console.error("Failed to read or parse local CODEOWNERS:", err.message || err); } // Hardcoded fallback list if CODEOWNERS could not be fetched or contained no reviewers (excluding author) From 1eaeb1bc9793d39f4a5609a1d5297f753fc667ca Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 10:11:16 -0700 Subject: [PATCH 03/12] refactor(workflow): simplify assign-reviewers script logic and helper abstractions --- .github/workflows/assign-reviewers.yml | 260 ++++++++----------------- 1 file changed, 77 insertions(+), 183 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index 9240575acf07..5b2d18e480b8 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -28,100 +28,64 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} script: | const fs = require('fs'); + const { owner, repo } = context.repo; + const pull_number = context.payload.pull_request.number; const author = context.payload.pull_request.user.login; - // Release PRs should remain the responsibility of the oncall, - // so we route them only to the default team rather than individual team members. - if (author === 'release-please[bot]' || author === 'release-please') { - console.log(`PR opened by ${author}. Ensuring cloud-sdk-nodejs-team is assigned as a reviewer.`); + const DEFAULT_TEAM = 'cloud-sdk-nodejs-team'; + const FALLBACK_TEAM = ['westarle', 'feywind', 'danieljbruce', 'shivanee-p']; + + const hasTeam = (teams, slug) => + teams.some(t => (t.slug || t.name || '').toLowerCase() === slug.toLowerCase()); + + const requestReviewers = (reviewers = [], team_reviewers = []) => + github.rest.pulls.requestReviewers({ owner, repo, pull_number, reviewers, team_reviewers }); + + const removeTeamReviewer = (team) => + github.rest.pulls.removeRequestedReviewers({ owner, repo, pull_number, reviewers: [], team_reviewers: [team] }); + + // 1. Release PRs route directly to default team + if (['release-please[bot]', 'release-please'].includes(author)) { const requestedTeams = context.payload.pull_request.requested_teams || []; - const hasDefaultTeam = requestedTeams.some(team => - (team.slug && team.slug.toLowerCase() === 'cloud-sdk-nodejs-team') || - (team.name && team.name.toLowerCase() === 'cloud-sdk-nodejs-team') - ); - if (!hasDefaultTeam) { + if (!hasTeam(requestedTeams, DEFAULT_TEAM)) { try { - await github.rest.pulls.requestReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - team_reviewers: ['cloud-sdk-nodejs-team'], - }); + await requestReviewers([], [DEFAULT_TEAM]); } catch (err) { - console.error("Failed to assign default cloud-sdk-nodejs-team reviewer:", err.message || err); + console.error(`Failed to assign ${DEFAULT_TEAM}:`, err.message || err); } } return; } + // 2. Check if PR already has reviewers, route-specific team, or reviews const requestedReviewers = context.payload.pull_request.requested_reviewers || []; const requestedTeams = context.payload.pull_request.requested_teams || []; + const hasDefaultTeam = hasTeam(requestedTeams, DEFAULT_TEAM); + const hasRouteTeam = requestedTeams.some(t => (t.slug || t.name || '').toLowerCase() !== DEFAULT_TEAM); - const hasDefaultTeam = requestedTeams.some(team => - (team.slug && team.slug.toLowerCase() === 'cloud-sdk-nodejs-team') || - (team.name && team.name.toLowerCase() === 'cloud-sdk-nodejs-team') - ); - const hasRouteSpecificTeam = requestedTeams.some(team => - (team.slug && team.slug.toLowerCase() !== 'cloud-sdk-nodejs-team') && - (!team.name || team.name.toLowerCase() !== 'cloud-sdk-nodejs-team') - ); - - // Check if PR has already been reviewed let hasReviews = false; try { - const { data: reviews } = await github.rest.pulls.listReviews({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - }); - if (reviews.length > 0) { - console.log("PR already has reviews."); - hasReviews = true; - } + const { data: reviews } = await github.rest.pulls.listReviews({ owner, repo, pull_number }); + hasReviews = reviews.length > 0; } catch (err) { console.warn("Failed to check PR reviews:", err.message || err); } - let shouldExit = false; - if (requestedReviewers.length > 0) { - console.log(`PR already has requested reviewers: ${requestedReviewers.map(r => r.login).join(', ')}.`); - shouldExit = true; - } else if (hasRouteSpecificTeam) { - console.log(`PR already has route-specific team reviewer requested: ${requestedTeams.map(t => t.slug || t.name).join(', ')}.`); - shouldExit = true; - } else if (hasReviews) { - shouldExit = true; - } - - if (shouldExit) { + if (requestedReviewers.length > 0 || hasRouteTeam || hasReviews) { if (hasDefaultTeam) { - console.log("PR already has individual, route-specific reviewers, or reviews, but default cloud-sdk-nodejs-team is still requested. Removing it..."); - try { - await github.rest.pulls.removeRequestedReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - reviewers: [], - team_reviewers: ['cloud-sdk-nodejs-team'], - }); - } catch (err) { - console.warn("Failed to remove default cloud-sdk-nodejs-team reviewer:", err.message || err); + console.log("PR already has reviewers/reviews; removing default team..."); + try { await removeTeamReviewer(DEFAULT_TEAM); } catch (err) { + console.warn(`Failed to remove ${DEFAULT_TEAM}:`, err.message || err); } } console.log("Skipping auto-assignment."); return; } - - // 2. Get list of files modified in the PR + // 3. Match modified files against path routing let files = []; try { - const res = await github.rest.pulls.listFiles({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - per_page: 100, - }); + const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); files = res.data; } catch (err) { console.error("Failed to list PR files:", err.message || err); @@ -138,164 +102,94 @@ jobs: { prefix: 'core/packages/google-auth-library-nodejs/', team: 'aion-team' } ]; - const assignedTeams = new Set(); - for (const route of PATH_ROUTING) { - if (files.some(file => file.filename.startsWith(route.prefix))) { - assignedTeams.add(route.team); - } - } + const matchedTeams = [...new Set( + PATH_ROUTING + .filter(route => files.some(f => f.filename.startsWith(route.prefix))) + .map(route => route.team) + )]; let assigned = false; - if (assignedTeams.size > 0) { - const teamReviewers = Array.from(assignedTeams); - console.log(`PR contains changes matching specific routes. Requesting review from: ${teamReviewers.join(', ')}`); + if (matchedTeams.length > 0) { + console.log(`Requesting review from route teams: ${matchedTeams.join(', ')}`); try { - await github.rest.pulls.requestReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - team_reviewers: teamReviewers, - }); + await requestReviewers([], matchedTeams); assigned = true; } catch (err) { - console.error(`Failed to assign route-specific team reviewers (${teamReviewers.join(', ')}):`, err.message || err); - console.log("Falling back to default team members."); + console.error(`Failed to assign team reviewers (${matchedTeams.join(', ')}):`, err.message || err); } } + // 4. Default reviewer assignment with load balancing if no route team was assigned if (!assigned) { - console.log("Reading .github/CODEOWNERS locally from repository..."); - let defaultReviewers = []; - try { - if (fs.existsSync('.github/CODEOWNERS')) { - const content = fs.readFileSync('.github/CODEOWNERS', 'utf-8'); - const lines = content.split('\n'); - for (const line of lines) { - const trimmed = line.trim(); - if (trimmed.startsWith('#')) { - const commentContent = trimmed.substring(1).trim(); - if (commentContent.startsWith('team_members:')) { - const listPart = commentContent.substring('team_members:'.length).trim(); - const parts = listPart.split(/\s+/); - defaultReviewers = parts - .filter(p => p.startsWith('@')) - .map(p => p.substring(1)) - .filter(login => login.toLowerCase() !== author.toLowerCase()); - break; - } + function getDefaultReviewers() { + try { + if (fs.existsSync('.github/CODEOWNERS')) { + const line = fs.readFileSync('.github/CODEOWNERS', 'utf-8') + .split('\n') + .map(l => l.trim()) + .find(l => l.startsWith('#') && l.includes('team_members:')); + if (line) { + const members = line.split('team_members:')[1].trim().split(/\s+/) + .filter(p => p.startsWith('@')).map(p => p.substring(1)) + .filter(login => login.toLowerCase() !== author.toLowerCase()); + if (members.length > 0) return members; } } - } else { - console.warn(".github/CODEOWNERS file not found locally."); + } catch (err) { + console.error("Failed to read CODEOWNERS:", err.message || err); } - } catch (err) { - console.error("Failed to read or parse local CODEOWNERS:", err.message || err); + return FALLBACK_TEAM.filter(login => login.toLowerCase() !== author.toLowerCase()); } - // Hardcoded fallback list if CODEOWNERS could not be fetched or contained no reviewers (excluding author) - if (defaultReviewers.length === 0) { - console.warn("No default reviewers found in CODEOWNERS. Using fallback team members."); - const FALLBACK_TEAM = ['pearigee', 'feywind', 'danieljbruce', 'shivanee-p']; - defaultReviewers = FALLBACK_TEAM.filter(login => login.toLowerCase() !== author.toLowerCase()); - } + const defaultReviewers = getDefaultReviewers(); if (defaultReviewers.length > 0) { - console.log(`Found default reviewers: ${defaultReviewers.join(', ')}. Load balancing among them.`); - let candidateLogins = defaultReviewers; - let minLoad = 0; - + let candidateQueue = [...defaultReviewers].sort(() => Math.random() - 0.5); try { - // Retrieve active open PRs to calculate load - const { data: openPRs } = await github.rest.pulls.list({ - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - per_page: 100, - }); - - const loadMap = {}; - for (const member of defaultReviewers) { - loadMap[member.toLowerCase()] = 0; - } + const { data: openPRs } = await github.rest.pulls.list({ owner, repo, state: 'open', per_page: 100 }); + const loadMap = Object.fromEntries(defaultReviewers.map(m => [m.toLowerCase(), 0])); for (const pr of openPRs) { - // Count pending review requests - if (pr.requested_reviewers) { - for (const reviewer of pr.requested_reviewers) { - const lower = (reviewer.login || '').toLowerCase(); - if (loadMap[lower] !== undefined) { - loadMap[lower]++; - } - } - } - // Count assignees - if (pr.assignees) { - for (const assignee of pr.assignees) { - const lower = (assignee.login || '').toLowerCase(); - if (loadMap[lower] !== undefined) { - loadMap[lower]++; - } - } + for (const user of [...(pr.requested_reviewers || []), ...(pr.assignees || [])]) { + const lower = (user.login || '').toLowerCase(); + if (lower in loadMap) loadMap[lower]++; } } - console.log("Current team workload:", loadMap); - // Find members with the minimum load - minLoad = Infinity; - candidateLogins = []; - for (const member of defaultReviewers) { - const load = loadMap[member.toLowerCase()]; - if (load < minLoad) { - minLoad = load; - candidateLogins = [member]; - } else if (load === minLoad) { - candidateLogins.push(member); - } - } + const minLoad = Math.min(...Object.values(loadMap)); + const leastLoaded = defaultReviewers.filter(m => loadMap[m.toLowerCase()] === minLoad); + const remaining = defaultReviewers.filter(m => loadMap[m.toLowerCase()] > minLoad); + + candidateQueue = [ + ...leastLoaded.sort(() => Math.random() - 0.5), + ...remaining.sort(() => Math.random() - 0.5) + ]; } catch (err) { - console.error("Failed to calculate workload from open PRs, proceeding with all default reviewers:", err.message || err); + console.error("Failed to calculate workload, proceeding with default list:", err.message || err); } - // Shuffle candidates and attempt to request review from least loaded candidates first, then any default reviewer - const candidateQueue = [ - ...candidateLogins.sort(() => Math.random() - 0.5), - ...defaultReviewers.filter(m => !candidateLogins.includes(m)).sort(() => Math.random() - 0.5) - ]; - for (const candidate of candidateQueue) { console.log(`Attempting to assign reviewer: ${candidate}`); try { - await github.rest.pulls.requestReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - reviewers: [candidate], - }); + await requestReviewers([candidate]); assigned = true; console.log(`Successfully assigned reviewer: ${candidate}`); break; } catch (reqErr) { - console.error(`Failed to assign reviewer ${candidate}:`, reqErr.message || reqErr); + console.error(`Failed to assign ${candidate}:`, reqErr.message || reqErr); } } } else { - console.warn("No default reviewers available to assign."); + console.warn("No default reviewers available."); } } + // 5. Remove default team if an individual or specific route reviewer was assigned if (assigned) { - console.log("Removing default cloud-sdk-nodejs-team reviewer..."); try { - await github.rest.pulls.removeRequestedReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number, - reviewers: [], - team_reviewers: ['cloud-sdk-nodejs-team'], - }); + await removeTeamReviewer(DEFAULT_TEAM); } catch (err) { - console.warn("Failed to remove default cloud-sdk-nodejs-team reviewer:", err.message || err); + console.warn(`Failed to remove default ${DEFAULT_TEAM}:`, err.message || err); } } - From de8642e165a570582d9301b3585a0cdc86695820 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 10:14:17 -0700 Subject: [PATCH 04/12] feat(workflow): make assign-reviewers workflow fully hermetic using local git diff and deterministic selection --- .github/workflows/assign-reviewers.yml | 82 ++++++++++---------------- 1 file changed, 32 insertions(+), 50 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index 5b2d18e480b8..a7d42e864fd8 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -20,6 +20,7 @@ jobs: uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: persist-credentials: false + fetch-depth: 0 - name: Assign Reviewers continue-on-error: true @@ -28,15 +29,17 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} script: | const fs = require('fs'); + const cp = require('child_process'); const { owner, repo } = context.repo; - const pull_number = context.payload.pull_request.number; - const author = context.payload.pull_request.user.login; + const pr = context.payload.pull_request; + const pull_number = pr.number; + const author = pr.user.login; const DEFAULT_TEAM = 'cloud-sdk-nodejs-team'; const FALLBACK_TEAM = ['westarle', 'feywind', 'danieljbruce', 'shivanee-p']; const hasTeam = (teams, slug) => - teams.some(t => (t.slug || t.name || '').toLowerCase() === slug.toLowerCase()); + (teams || []).some(t => (t.slug || t.name || '').toLowerCase() === slug.toLowerCase()); const requestReviewers = (reviewers = [], team_reviewers = []) => github.rest.pulls.requestReviewers({ owner, repo, pull_number, reviewers, team_reviewers }); @@ -46,8 +49,7 @@ jobs: // 1. Release PRs route directly to default team if (['release-please[bot]', 'release-please'].includes(author)) { - const requestedTeams = context.payload.pull_request.requested_teams || []; - if (!hasTeam(requestedTeams, DEFAULT_TEAM)) { + if (!hasTeam(pr.requested_teams, DEFAULT_TEAM)) { try { await requestReviewers([], [DEFAULT_TEAM]); } catch (err) { @@ -57,23 +59,14 @@ jobs: return; } - // 2. Check if PR already has reviewers, route-specific team, or reviews - const requestedReviewers = context.payload.pull_request.requested_reviewers || []; - const requestedTeams = context.payload.pull_request.requested_teams || []; - const hasDefaultTeam = hasTeam(requestedTeams, DEFAULT_TEAM); - const hasRouteTeam = requestedTeams.some(t => (t.slug || t.name || '').toLowerCase() !== DEFAULT_TEAM); + // 2. Check if PR already has requested reviewers or route-specific team + const hasDefaultTeam = hasTeam(pr.requested_teams, DEFAULT_TEAM); + const hasRouteTeam = (pr.requested_teams || []).some(t => (t.slug || t.name || '').toLowerCase() !== DEFAULT_TEAM); + const hasRequestedReviewers = (pr.requested_reviewers || []).length > 0; - let hasReviews = false; - try { - const { data: reviews } = await github.rest.pulls.listReviews({ owner, repo, pull_number }); - hasReviews = reviews.length > 0; - } catch (err) { - console.warn("Failed to check PR reviews:", err.message || err); - } - - if (requestedReviewers.length > 0 || hasRouteTeam || hasReviews) { + if (hasRequestedReviewers || hasRouteTeam) { if (hasDefaultTeam) { - console.log("PR already has reviewers/reviews; removing default team..."); + console.log("PR already has reviewers assigned; removing default team..."); try { await removeTeamReviewer(DEFAULT_TEAM); } catch (err) { console.warn(`Failed to remove ${DEFAULT_TEAM}:`, err.message || err); } @@ -82,13 +75,20 @@ jobs: return; } - // 3. Match modified files against path routing + // 3. Obtain changed files hermetically from local git checkout let files = []; try { - const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); - files = res.data; + const baseRef = pr.base.ref || 'main'; + const diffOutput = cp.execSync(`git diff --name-only origin/${baseRef}...HEAD`, { encoding: 'utf-8' }); + files = diffOutput.split('\n').filter(Boolean); } catch (err) { - console.error("Failed to list PR files:", err.message || err); + console.warn("Could not get files from git diff, falling back to API:", err.message || err); + try { + const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); + files = res.data.map(f => f.filename); + } catch (apiErr) { + console.error("Failed to list PR files:", apiErr.message || apiErr); + } } const PATH_ROUTING = [ @@ -104,7 +104,7 @@ jobs: const matchedTeams = [...new Set( PATH_ROUTING - .filter(route => files.some(f => f.filename.startsWith(route.prefix))) + .filter(route => files.some(file => file.startsWith(route.prefix))) .map(route => route.team) )]; @@ -119,7 +119,7 @@ jobs: } } - // 4. Default reviewer assignment with load balancing if no route team was assigned + // 4. Deterministic default reviewer assignment from local CODEOWNERS if (!assigned) { function getDefaultReviewers() { try { @@ -144,30 +144,12 @@ jobs: const defaultReviewers = getDefaultReviewers(); if (defaultReviewers.length > 0) { - let candidateQueue = [...defaultReviewers].sort(() => Math.random() - 0.5); - try { - const { data: openPRs } = await github.rest.pulls.list({ owner, repo, state: 'open', per_page: 100 }); - const loadMap = Object.fromEntries(defaultReviewers.map(m => [m.toLowerCase(), 0])); - - for (const pr of openPRs) { - for (const user of [...(pr.requested_reviewers || []), ...(pr.assignees || [])]) { - const lower = (user.login || '').toLowerCase(); - if (lower in loadMap) loadMap[lower]++; - } - } - console.log("Current team workload:", loadMap); - - const minLoad = Math.min(...Object.values(loadMap)); - const leastLoaded = defaultReviewers.filter(m => loadMap[m.toLowerCase()] === minLoad); - const remaining = defaultReviewers.filter(m => loadMap[m.toLowerCase()] > minLoad); - - candidateQueue = [ - ...leastLoaded.sort(() => Math.random() - 0.5), - ...remaining.sort(() => Math.random() - 0.5) - ]; - } catch (err) { - console.error("Failed to calculate workload, proceeding with default list:", err.message || err); - } + // Deterministic selection based on PR number for even load distribution without external API state calls + const selectedIndex = pull_number % defaultReviewers.length; + const candidateQueue = [ + defaultReviewers[selectedIndex], + ...defaultReviewers.filter((_, i) => i !== selectedIndex) + ]; for (const candidate of candidateQueue) { console.log(`Attempting to assign reviewer: ${candidate}`); From 3744b2c17200589d880d5b5ddd6b945e3301c943 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 10:21:55 -0700 Subject: [PATCH 05/12] fix(workflow): remove empty reviewers array from removeRequestedReviewers payload --- .github/workflows/assign-reviewers.yml | 30 +++++++++++++++++--------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index a7d42e864fd8..2b1f0d42d32c 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -41,11 +41,20 @@ jobs: const hasTeam = (teams, slug) => (teams || []).some(t => (t.slug || t.name || '').toLowerCase() === slug.toLowerCase()); - const requestReviewers = (reviewers = [], team_reviewers = []) => - github.rest.pulls.requestReviewers({ owner, repo, pull_number, reviewers, team_reviewers }); + const requestReviewers = (reviewers = [], team_reviewers = []) => { + const payload = { owner, repo, pull_number }; + if (reviewers.length > 0) payload.reviewers = reviewers; + if (team_reviewers.length > 0) payload.team_reviewers = team_reviewers; + return github.rest.pulls.requestReviewers(payload); + }; const removeTeamReviewer = (team) => - github.rest.pulls.removeRequestedReviewers({ owner, repo, pull_number, reviewers: [], team_reviewers: [team] }); + github.rest.pulls.removeRequestedReviewers({ + owner, + repo, + pull_number, + team_reviewers: [team] + }); // 1. Release PRs route directly to default team if (['release-please[bot]', 'release-please'].includes(author)) { @@ -60,16 +69,15 @@ jobs: } // 2. Check if PR already has requested reviewers or route-specific team - const hasDefaultTeam = hasTeam(pr.requested_teams, DEFAULT_TEAM); const hasRouteTeam = (pr.requested_teams || []).some(t => (t.slug || t.name || '').toLowerCase() !== DEFAULT_TEAM); const hasRequestedReviewers = (pr.requested_reviewers || []).length > 0; if (hasRequestedReviewers || hasRouteTeam) { - if (hasDefaultTeam) { - console.log("PR already has reviewers assigned; removing default team..."); - try { await removeTeamReviewer(DEFAULT_TEAM); } catch (err) { - console.warn(`Failed to remove ${DEFAULT_TEAM}:`, err.message || err); - } + console.log("PR already has reviewers assigned; ensuring default team is removed..."); + try { + await removeTeamReviewer(DEFAULT_TEAM); + } catch (err) { + console.warn(`Failed to remove ${DEFAULT_TEAM}:`, err.message || err); } console.log("Skipping auto-assignment."); return; @@ -169,9 +177,11 @@ jobs: // 5. Remove default team if an individual or specific route reviewer was assigned if (assigned) { + console.log(`Removing default team (${DEFAULT_TEAM})...`); try { await removeTeamReviewer(DEFAULT_TEAM); + console.log(`Successfully removed default team (${DEFAULT_TEAM}).`); } catch (err) { - console.warn(`Failed to remove default ${DEFAULT_TEAM}:`, err.message || err); + console.error(`Failed to remove default team (${DEFAULT_TEAM}):`, err.message || err); } } From 4a4be7518091584033b5d9e4b6a41097c33f1e9a Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 12:00:51 -0700 Subject: [PATCH 06/12] fix(ci): pass GIT_DIFF_ARG env variable in assign-reviewers workflow step --- .github/workflows/assign-reviewers.yml | 82 ++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 10 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index 2b1f0d42d32c..50435cfae777 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -25,6 +25,8 @@ jobs: - name: Assign Reviewers continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 + env: + GIT_DIFF_ARG: "origin/${{ github.base_ref }}...HEAD" with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | @@ -85,17 +87,77 @@ jobs: // 3. Obtain changed files hermetically from local git checkout let files = []; - try { - const baseRef = pr.base.ref || 'main'; - const diffOutput = cp.execSync(`git diff --name-only origin/${baseRef}...HEAD`, { encoding: 'utf-8' }); - files = diffOutput.split('\n').filter(Boolean); - } catch (err) { - console.warn("Could not get files from git diff, falling back to API:", err.message || err); + const baseRef = pr.base.ref || 'main'; + + const getGitDiffArg = () => { + const cliIndex = process.argv.findIndex(arg => arg.startsWith('--git-diff-arg')); + if (cliIndex !== -1) { + const arg = process.argv[cliIndex]; + if (arg.includes('=')) { + return arg.substring(arg.indexOf('=') + 1); + } + if (process.argv[cliIndex + 1] && !process.argv[cliIndex + 1].startsWith('-')) { + return process.argv[cliIndex + 1]; + } + } + return process.env.GIT_DIFF_ARG; + }; + + const isStrict = process.argv.includes('--strict') || process.env.STRICT === 'true' || Boolean(process.env.GIT_DIFF_ARG); + const gitDiffArg = getGitDiffArg(); + + if (isStrict) { + if (!gitDiffArg) { + throw new Error( + 'Strict mode is enabled (--strict), but GIT_DIFF_ARG was not provided. ' + + 'Please supply --git-diff-arg or set the GIT_DIFF_ARG environment variable.' + ); + } try { - const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); - files = res.data.map(f => f.filename); - } catch (apiErr) { - console.error("Failed to list PR files:", apiErr.message || apiErr); + cp.execSync(`git diff --quiet ${gitDiffArg}`); + } catch (err) { + if (err.status !== 1) { + throw new Error( + `Strict mode error: git diff --quiet ${gitDiffArg} failed with exit code ${err.status}.\n` + + `Ensure that the git reference '${gitDiffArg}' exists locally and that you have fetched the required commits/branches.\n` + + `Details: ${String(err.stderr || err.message || '').trim()}` + ); + } + } + const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${gitDiffArg}`, { encoding: 'utf-8' }); + files = diffOutput.split('\n').map(f => f.trim()).filter(Boolean); + console.log(`Strict mode enabled. Comparing using GIT_DIFF_ARG: ${gitDiffArg}`); + } else { + const refsToTry = baseRef.startsWith('origin/') ? [baseRef] : [`origin/${baseRef}`, baseRef]; + + for (const ref of refsToTry) { + try { + let diffTarget = ref; + try { + const mergeBase = cp.execSync(`git merge-base ${ref} HEAD`, { encoding: 'utf-8' }).trim(); + if (mergeBase) { + diffTarget = mergeBase; + } + } catch { + // Fall back to ref directly if merge-base fails + } + const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${diffTarget}`, { encoding: 'utf-8' }); + files = diffOutput.split('\n').map(f => f.trim()).filter(Boolean); + console.log(`Comparing against base reference: ${ref}`); + break; + } catch (err) { + console.warn(`Could not get files from git diff against ${ref}:`, err.message || err); + } + } + + if (files.length === 0) { + try { + console.log("Falling back to GitHub REST API to list PR files..."); + const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); + files = res.data.map(f => f.filename); + } catch (apiErr) { + console.error("Failed to list PR files via API:", apiErr.message || apiErr); + } } } From 1b7a43697817cbc726e1fee82aedff8ba3051936 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 12:00:51 -0700 Subject: [PATCH 07/12] fix(ci): fetch base branch in assign-reviewers workflow for strict diff --- .github/workflows/assign-reviewers.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index 50435cfae777..ac1cfdc7522c 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -22,6 +22,9 @@ jobs: persist-credentials: false fetch-depth: 0 + - name: Fetch base branch for diff + run: git fetch origin ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }} + - name: Assign Reviewers continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 From bf954d90a560d0af2b8a31765fbc0f60a4d8f9dd Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Fri, 31 Jul 2026 14:04:29 -0700 Subject: [PATCH 08/12] refactor(ci): use STRICT environment variable for strict mode in assign-reviewers workflow --- .github/workflows/assign-reviewers.yml | 34 +++++--------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index ac1cfdc7522c..e51ca175b767 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -29,6 +29,7 @@ jobs: continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 env: + STRICT: "true" GIT_DIFF_ARG: "origin/${{ github.base_ref }}...HEAD" with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -92,28 +93,14 @@ jobs: let files = []; const baseRef = pr.base.ref || 'main'; - const getGitDiffArg = () => { - const cliIndex = process.argv.findIndex(arg => arg.startsWith('--git-diff-arg')); - if (cliIndex !== -1) { - const arg = process.argv[cliIndex]; - if (arg.includes('=')) { - return arg.substring(arg.indexOf('=') + 1); - } - if (process.argv[cliIndex + 1] && !process.argv[cliIndex + 1].startsWith('-')) { - return process.argv[cliIndex + 1]; - } - } - return process.env.GIT_DIFF_ARG; - }; - - const isStrict = process.argv.includes('--strict') || process.env.STRICT === 'true' || Boolean(process.env.GIT_DIFF_ARG); - const gitDiffArg = getGitDiffArg(); + const isStrict = Boolean(process.env.STRICT); if (isStrict) { + const gitDiffArg = process.env.GIT_DIFF_ARG; if (!gitDiffArg) { throw new Error( - 'Strict mode is enabled (--strict), but GIT_DIFF_ARG was not provided. ' + - 'Please supply --git-diff-arg or set the GIT_DIFF_ARG environment variable.' + 'Strict mode is enabled, but GIT_DIFF_ARG environment variable was not provided. ' + + 'Please set the GIT_DIFF_ARG environment variable.' ); } try { @@ -135,16 +122,7 @@ jobs: for (const ref of refsToTry) { try { - let diffTarget = ref; - try { - const mergeBase = cp.execSync(`git merge-base ${ref} HEAD`, { encoding: 'utf-8' }).trim(); - if (mergeBase) { - diffTarget = mergeBase; - } - } catch { - // Fall back to ref directly if merge-base fails - } - const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${diffTarget}`, { encoding: 'utf-8' }); + const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${ref}...HEAD`, { encoding: 'utf-8' }); files = diffOutput.split('\n').map(f => f.trim()).filter(Boolean); console.log(`Comparing against base reference: ${ref}`); break; From 6b990dedbe57c6d7f8ace448850d1dee52c675dd Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Mon, 3 Aug 2026 10:01:52 -0700 Subject: [PATCH 09/12] fix(ci): align strict mode diff logic in assign-reviewers workflow with PR #9021 --- .github/workflows/assign-reviewers.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index e51ca175b767..6ebc30ab04ff 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -20,17 +20,14 @@ jobs: uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: persist-credentials: false - fetch-depth: 0 - - - name: Fetch base branch for diff - run: git fetch origin ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }} + fetch-depth: 2 - name: Assign Reviewers continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 env: STRICT: "true" - GIT_DIFF_ARG: "origin/${{ github.base_ref }}...HEAD" + GIT_DIFF_ARG: "HEAD^1" with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | From 2e99ff9a0e72f907a858b609e051703d37713b5c Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Mon, 3 Aug 2026 16:16:03 -0700 Subject: [PATCH 10/12] fix(ci): check for default team before removing and consolidate script logic --- .github/workflows/assign-reviewers.yml | 121 ++++++++++++------------- 1 file changed, 59 insertions(+), 62 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index 6ebc30ab04ff..3c45e55593c8 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -51,13 +51,22 @@ jobs: return github.rest.pulls.requestReviewers(payload); }; - const removeTeamReviewer = (team) => - github.rest.pulls.removeRequestedReviewers({ - owner, - repo, - pull_number, - team_reviewers: [team] - }); + const removeDefaultTeamIfRequested = async () => { + if (hasTeam(pr.requested_teams, DEFAULT_TEAM)) { + console.log(`Removing default team (${DEFAULT_TEAM})...`); + try { + await github.rest.pulls.removeRequestedReviewers({ + owner, + repo, + pull_number, + team_reviewers: [DEFAULT_TEAM] + }); + console.log(`Successfully removed default team (${DEFAULT_TEAM}).`); + } catch (err) { + console.warn(`Failed to remove default team (${DEFAULT_TEAM}):`, err.message || err); + } + } + }; // 1. Release PRs route directly to default team if (['release-please[bot]', 'release-please'].includes(author)) { @@ -71,73 +80,66 @@ jobs: return; } - // 2. Check if PR already has requested reviewers or route-specific team + // 2. Skip auto-assignment if reviewers or route-specific team are already requested const hasRouteTeam = (pr.requested_teams || []).some(t => (t.slug || t.name || '').toLowerCase() !== DEFAULT_TEAM); const hasRequestedReviewers = (pr.requested_reviewers || []).length > 0; if (hasRequestedReviewers || hasRouteTeam) { - console.log("PR already has reviewers assigned; ensuring default team is removed..."); - try { - await removeTeamReviewer(DEFAULT_TEAM); - } catch (err) { - console.warn(`Failed to remove ${DEFAULT_TEAM}:`, err.message || err); - } + console.log("PR already has reviewers assigned; ensuring default team is removed if needed..."); + await removeDefaultTeamIfRequested(); console.log("Skipping auto-assignment."); return; } - // 3. Obtain changed files hermetically from local git checkout - let files = []; - const baseRef = pr.base.ref || 'main'; - - const isStrict = Boolean(process.env.STRICT); - - if (isStrict) { - const gitDiffArg = process.env.GIT_DIFF_ARG; - if (!gitDiffArg) { - throw new Error( - 'Strict mode is enabled, but GIT_DIFF_ARG environment variable was not provided. ' + - 'Please set the GIT_DIFF_ARG environment variable.' - ); - } - try { - cp.execSync(`git diff --quiet ${gitDiffArg}`); - } catch (err) { - if (err.status !== 1) { + // 3. Obtain changed files (via local git diff or API fallback) + const getChangedFiles = async () => { + const isStrict = Boolean(process.env.STRICT); + if (isStrict) { + const gitDiffArg = process.env.GIT_DIFF_ARG; + if (!gitDiffArg) { throw new Error( - `Strict mode error: git diff --quiet ${gitDiffArg} failed with exit code ${err.status}.\n` + - `Ensure that the git reference '${gitDiffArg}' exists locally and that you have fetched the required commits/branches.\n` + - `Details: ${String(err.stderr || err.message || '').trim()}` + 'Strict mode is enabled, but GIT_DIFF_ARG environment variable was not provided.' ); } + try { + cp.execSync(`git diff --quiet ${gitDiffArg}`); + } catch (err) { + if (err.status !== 1) { + throw new Error( + `Strict mode error: git diff --quiet ${gitDiffArg} failed with exit code ${err.status}.\n` + + `Details: ${String(err.stderr || err.message || '').trim()}` + ); + } + } + const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${gitDiffArg}`, { encoding: 'utf-8' }); + console.log(`Strict mode enabled. Comparing using GIT_DIFF_ARG: ${gitDiffArg}`); + return diffOutput.split('\n').map(f => f.trim()).filter(Boolean); } - const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${gitDiffArg}`, { encoding: 'utf-8' }); - files = diffOutput.split('\n').map(f => f.trim()).filter(Boolean); - console.log(`Strict mode enabled. Comparing using GIT_DIFF_ARG: ${gitDiffArg}`); - } else { + + const baseRef = pr.base.ref || 'main'; const refsToTry = baseRef.startsWith('origin/') ? [baseRef] : [`origin/${baseRef}`, baseRef]; for (const ref of refsToTry) { try { const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${ref}...HEAD`, { encoding: 'utf-8' }); - files = diffOutput.split('\n').map(f => f.trim()).filter(Boolean); console.log(`Comparing against base reference: ${ref}`); - break; + return diffOutput.split('\n').map(f => f.trim()).filter(Boolean); } catch (err) { console.warn(`Could not get files from git diff against ${ref}:`, err.message || err); } } - if (files.length === 0) { - try { - console.log("Falling back to GitHub REST API to list PR files..."); - const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); - files = res.data.map(f => f.filename); - } catch (apiErr) { - console.error("Failed to list PR files via API:", apiErr.message || apiErr); - } + try { + console.log("Falling back to GitHub REST API to list PR files..."); + const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); + return res.data.map(f => f.filename); + } catch (apiErr) { + console.error("Failed to list PR files via API:", apiErr.message || apiErr); + return []; } - } + }; + + const files = await getChangedFiles(); const PATH_ROUTING = [ { prefix: 'handwritten/bigquery/', team: 'bigquery-team' }, @@ -157,6 +159,8 @@ jobs: )]; let assigned = false; + + // 4. Request review from route-specific teams if applicable if (matchedTeams.length > 0) { console.log(`Requesting review from route teams: ${matchedTeams.join(', ')}`); try { @@ -167,9 +171,9 @@ jobs: } } - // 4. Deterministic default reviewer assignment from local CODEOWNERS + // 5. Fallback to deterministic default reviewer assignment from local CODEOWNERS if (!assigned) { - function getDefaultReviewers() { + const getDefaultReviewers = () => { try { if (fs.existsSync('.github/CODEOWNERS')) { const line = fs.readFileSync('.github/CODEOWNERS', 'utf-8') @@ -187,12 +191,11 @@ jobs: console.error("Failed to read CODEOWNERS:", err.message || err); } return FALLBACK_TEAM.filter(login => login.toLowerCase() !== author.toLowerCase()); - } + }; const defaultReviewers = getDefaultReviewers(); if (defaultReviewers.length > 0) { - // Deterministic selection based on PR number for even load distribution without external API state calls const selectedIndex = pull_number % defaultReviewers.length; const candidateQueue = [ defaultReviewers[selectedIndex], @@ -215,13 +218,7 @@ jobs: } } - // 5. Remove default team if an individual or specific route reviewer was assigned + // 6. Remove default team if an individual or specific route reviewer was assigned if (assigned) { - console.log(`Removing default team (${DEFAULT_TEAM})...`); - try { - await removeTeamReviewer(DEFAULT_TEAM); - console.log(`Successfully removed default team (${DEFAULT_TEAM}).`); - } catch (err) { - console.error(`Failed to remove default team (${DEFAULT_TEAM}):`, err.message || err); - } + await removeDefaultTeamIfRequested(); } From 19b874c56b50d491d29c4c146e2bf16a653332c9 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Mon, 3 Aug 2026 16:17:22 -0700 Subject: [PATCH 11/12] fix(ci): check for default team alias before removing and remove strict mode logic --- .github/workflows/assign-reviewers.yml | 28 +------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index 3c45e55593c8..fa094d5a5a1f 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -20,14 +20,11 @@ jobs: uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: persist-credentials: false - fetch-depth: 2 + fetch-depth: 0 - name: Assign Reviewers continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 - env: - STRICT: "true" - GIT_DIFF_ARG: "HEAD^1" with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | @@ -93,29 +90,6 @@ jobs: // 3. Obtain changed files (via local git diff or API fallback) const getChangedFiles = async () => { - const isStrict = Boolean(process.env.STRICT); - if (isStrict) { - const gitDiffArg = process.env.GIT_DIFF_ARG; - if (!gitDiffArg) { - throw new Error( - 'Strict mode is enabled, but GIT_DIFF_ARG environment variable was not provided.' - ); - } - try { - cp.execSync(`git diff --quiet ${gitDiffArg}`); - } catch (err) { - if (err.status !== 1) { - throw new Error( - `Strict mode error: git diff --quiet ${gitDiffArg} failed with exit code ${err.status}.\n` + - `Details: ${String(err.stderr || err.message || '').trim()}` - ); - } - } - const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${gitDiffArg}`, { encoding: 'utf-8' }); - console.log(`Strict mode enabled. Comparing using GIT_DIFF_ARG: ${gitDiffArg}`); - return diffOutput.split('\n').map(f => f.trim()).filter(Boolean); - } - const baseRef = pr.base.ref || 'main'; const refsToTry = baseRef.startsWith('origin/') ? [baseRef] : [`origin/${baseRef}`, baseRef]; From f346600dd117b339adadcbd2445278832b09e4d2 Mon Sep 17 00:00:00 2001 From: Shivanee Persaud Date: Mon, 3 Aug 2026 16:22:23 -0700 Subject: [PATCH 12/12] revert: restore assign-reviewers.yml to main branch version --- .github/workflows/assign-reviewers.yml | 302 +++++++++++++++---------- 1 file changed, 180 insertions(+), 122 deletions(-) diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index fa094d5a5a1f..a917a32d7bb3 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -16,104 +16,94 @@ jobs: runs-on: ubuntu-latest if: github.event.pull_request.draft == false steps: - - name: Checkout repository - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 - with: - persist-credentials: false - fetch-depth: 0 - - name: Assign Reviewers continue-on-error: true uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const fs = require('fs'); - const cp = require('child_process'); - const { owner, repo } = context.repo; - const pr = context.payload.pull_request; - const pull_number = pr.number; - const author = pr.user.login; - - const DEFAULT_TEAM = 'cloud-sdk-nodejs-team'; - const FALLBACK_TEAM = ['westarle', 'feywind', 'danieljbruce', 'shivanee-p']; - - const hasTeam = (teams, slug) => - (teams || []).some(t => (t.slug || t.name || '').toLowerCase() === slug.toLowerCase()); - - const requestReviewers = (reviewers = [], team_reviewers = []) => { - const payload = { owner, repo, pull_number }; - if (reviewers.length > 0) payload.reviewers = reviewers; - if (team_reviewers.length > 0) payload.team_reviewers = team_reviewers; - return github.rest.pulls.requestReviewers(payload); - }; - - const removeDefaultTeamIfRequested = async () => { - if (hasTeam(pr.requested_teams, DEFAULT_TEAM)) { - console.log(`Removing default team (${DEFAULT_TEAM})...`); - try { - await github.rest.pulls.removeRequestedReviewers({ - owner, - repo, - pull_number, - team_reviewers: [DEFAULT_TEAM] - }); - console.log(`Successfully removed default team (${DEFAULT_TEAM}).`); - } catch (err) { - console.warn(`Failed to remove default team (${DEFAULT_TEAM}):`, err.message || err); - } - } - }; + const author = context.payload.pull_request.user.login; - // 1. Release PRs route directly to default team - if (['release-please[bot]', 'release-please'].includes(author)) { - if (!hasTeam(pr.requested_teams, DEFAULT_TEAM)) { + // Release PRs should remain the responsibility of the oncall, + // so we route them only to the default team rather than individual team members. + if (author === 'release-please[bot]' || author === 'release-please') { + console.log(`PR opened by ${author}. Ensuring cloud-sdk-nodejs-team is assigned as a reviewer.`); + const requestedTeams = context.payload.pull_request.requested_teams || []; + const hasDefaultTeam = requestedTeams.some(team => team.slug === 'cloud-sdk-nodejs-team'); + if (!hasDefaultTeam) { try { - await requestReviewers([], [DEFAULT_TEAM]); + await github.rest.pulls.requestReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + team_reviewers: ['cloud-sdk-nodejs-team'], + }); } catch (err) { - console.error(`Failed to assign ${DEFAULT_TEAM}:`, err.message || err); + console.error("Failed to assign default cloud-sdk-nodejs-team reviewer:", err.message || err); } } return; } - // 2. Skip auto-assignment if reviewers or route-specific team are already requested - const hasRouteTeam = (pr.requested_teams || []).some(t => (t.slug || t.name || '').toLowerCase() !== DEFAULT_TEAM); - const hasRequestedReviewers = (pr.requested_reviewers || []).length > 0; + const requestedReviewers = context.payload.pull_request.requested_reviewers || []; + const requestedTeams = context.payload.pull_request.requested_teams || []; - if (hasRequestedReviewers || hasRouteTeam) { - console.log("PR already has reviewers assigned; ensuring default team is removed if needed..."); - await removeDefaultTeamIfRequested(); - console.log("Skipping auto-assignment."); - return; + const hasDefaultTeam = requestedTeams.some(team => team.slug === 'cloud-sdk-nodejs-team'); + const hasRouteSpecificTeam = requestedTeams.some(team => team.slug !== 'cloud-sdk-nodejs-team'); + + // Check if PR has already been reviewed + let hasReviews = false; + try { + const { data: reviews } = await github.rest.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + }); + if (reviews.length > 0) { + console.log("PR already has reviews."); + hasReviews = true; + } + } catch (err) { + console.warn("Failed to check PR reviews:", err.message || err); } - // 3. Obtain changed files (via local git diff or API fallback) - const getChangedFiles = async () => { - const baseRef = pr.base.ref || 'main'; - const refsToTry = baseRef.startsWith('origin/') ? [baseRef] : [`origin/${baseRef}`, baseRef]; + let shouldExit = false; + if (requestedReviewers.length > 0) { + console.log(`PR already has requested reviewers: ${requestedReviewers.map(r => r.login).join(', ')}.`); + shouldExit = true; + } else if (hasRouteSpecificTeam) { + console.log(`PR already has route-specific team reviewer requested: ${requestedTeams.map(t => t.slug).join(', ')}.`); + shouldExit = true; + } else if (hasReviews) { + shouldExit = true; + } - for (const ref of refsToTry) { + if (shouldExit) { + if (hasDefaultTeam) { + console.log("PR already has individual, route-specific reviewers, or reviews, but default cloud-sdk-nodejs-team is still requested. Removing it..."); try { - const diffOutput = cp.execSync(`git diff --name-only --diff-filter=ACMRT ${ref}...HEAD`, { encoding: 'utf-8' }); - console.log(`Comparing against base reference: ${ref}`); - return diffOutput.split('\n').map(f => f.trim()).filter(Boolean); + await github.rest.pulls.removeRequestedReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + reviewers: [], + team_reviewers: ['cloud-sdk-nodejs-team'], + }); } catch (err) { - console.warn(`Could not get files from git diff against ${ref}:`, err.message || err); + console.warn("Failed to remove default cloud-sdk-nodejs-team reviewer:", err.message || err); } } + console.log("Skipping auto-assignment."); + return; + } - try { - console.log("Falling back to GitHub REST API to list PR files..."); - const res = await github.rest.pulls.listFiles({ owner, repo, pull_number, per_page: 100 }); - return res.data.map(f => f.filename); - } catch (apiErr) { - console.error("Failed to list PR files via API:", apiErr.message || apiErr); - return []; - } - }; - const files = await getChangedFiles(); + // 2. Get list of files modified in the PR + const { data: files } = await github.rest.pulls.listFiles({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + }); const PATH_ROUTING = [ { prefix: 'handwritten/bigquery/', team: 'bigquery-team' }, @@ -126,73 +116,141 @@ jobs: { prefix: 'core/packages/google-auth-library-nodejs/', team: 'aion-team' } ]; - const matchedTeams = [...new Set( - PATH_ROUTING - .filter(route => files.some(file => file.startsWith(route.prefix))) - .map(route => route.team) - )]; + const assignedTeams = new Set(); + for (const route of PATH_ROUTING) { + if (files.some(file => file.filename.startsWith(route.prefix))) { + assignedTeams.add(route.team); + } + } let assigned = false; - - // 4. Request review from route-specific teams if applicable - if (matchedTeams.length > 0) { - console.log(`Requesting review from route teams: ${matchedTeams.join(', ')}`); + if (assignedTeams.size > 0) { + const teamReviewers = Array.from(assignedTeams); + console.log(`PR contains changes matching specific routes. Requesting review from: ${teamReviewers.join(', ')}`); try { - await requestReviewers([], matchedTeams); + await github.rest.pulls.requestReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + team_reviewers: teamReviewers, + }); assigned = true; } catch (err) { - console.error(`Failed to assign team reviewers (${matchedTeams.join(', ')}):`, err.message || err); + console.error(`Failed to assign route-specific team reviewers (${teamReviewers.join(', ')}):`, err.message || err); + console.log("Falling back to default team members."); } } - // 5. Fallback to deterministic default reviewer assignment from local CODEOWNERS if (!assigned) { - const getDefaultReviewers = () => { - try { - if (fs.existsSync('.github/CODEOWNERS')) { - const line = fs.readFileSync('.github/CODEOWNERS', 'utf-8') - .split('\n') - .map(l => l.trim()) - .find(l => l.startsWith('#') && l.includes('team_members:')); - if (line) { - const members = line.split('team_members:')[1].trim().split(/\s+/) - .filter(p => p.startsWith('@')).map(p => p.substring(1)) - .filter(login => login.toLowerCase() !== author.toLowerCase()); - if (members.length > 0) return members; + console.log("Fetching .github/CODEOWNERS to identify default reviewers..."); + let defaultReviewers = []; + try { + const { data: codeownersData } = await github.rest.repos.getContent({ + owner: context.repo.owner, + repo: context.repo.repo, + path: '.github/CODEOWNERS', + ref: context.payload.pull_request.head.sha, + }); + const content = Buffer.from(codeownersData.content, 'base64').toString('utf-8'); + const lines = content.split('\n'); + for (const line of lines) { + const trimmed = line.trim(); + if (trimmed.startsWith('#')) { + const commentContent = trimmed.substring(1).trim(); + if (commentContent.startsWith('team_members:')) { + const listPart = commentContent.substring('team_members:'.length).trim(); + const parts = listPart.split(/\s+/); + defaultReviewers = parts + .filter(p => p.startsWith('@')) + .map(p => p.substring(1)) + .filter(login => login !== author); + break; } } - } catch (err) { - console.error("Failed to read CODEOWNERS:", err.message || err); } - return FALLBACK_TEAM.filter(login => login.toLowerCase() !== author.toLowerCase()); - }; - - const defaultReviewers = getDefaultReviewers(); + } catch (err) { + console.error("Failed to read or parse CODEOWNERS:", err); + } if (defaultReviewers.length > 0) { - const selectedIndex = pull_number % defaultReviewers.length; - const candidateQueue = [ - defaultReviewers[selectedIndex], - ...defaultReviewers.filter((_, i) => i !== selectedIndex) - ]; - - for (const candidate of candidateQueue) { - console.log(`Attempting to assign reviewer: ${candidate}`); - try { - await requestReviewers([candidate]); - assigned = true; - console.log(`Successfully assigned reviewer: ${candidate}`); - break; - } catch (reqErr) { - console.error(`Failed to assign ${candidate}:`, reqErr.message || reqErr); + console.log(`Found default reviewers: ${defaultReviewers.join(', ')}. Load balancing among them.`); + try { + // Retrieve active open PRs to calculate load + const { data: openPRs } = await github.rest.pulls.list({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'open', + per_page: 100, + }); + + const loadMap = {}; + for (const member of defaultReviewers) { + loadMap[member] = 0; + } + + for (const pr of openPRs) { + // Count pending review requests + if (pr.requested_reviewers) { + for (const reviewer of pr.requested_reviewers) { + if (loadMap[reviewer.login] !== undefined) { + loadMap[reviewer.login]++; + } + } + } + // Count assignees + if (pr.assignees) { + for (const assignee of pr.assignees) { + if (loadMap[assignee.login] !== undefined) { + loadMap[assignee.login]++; + } + } + } + } + + console.log("Current team workload:", loadMap); + + // Find members with the minimum load + let minLoad = Infinity; + let selectedReviewers = []; + for (const member of defaultReviewers) { + const load = loadMap[member]; + if (load < minLoad) { + minLoad = load; + selectedReviewers = [member]; + } else if (load === minLoad) { + selectedReviewers.push(member); + } } + + const leastLoadedReviewer = selectedReviewers[Math.floor(Math.random() * selectedReviewers.length)]; + console.log(`Selected reviewer with least load (load: ${minLoad}): ${leastLoadedReviewer}`); + + await github.rest.pulls.requestReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + reviewers: [leastLoadedReviewer], + }); + assigned = true; + } catch (err) { + console.error("Failed to assign reviewers using load balancing:", err); } } else { - console.warn("No default reviewers available."); + console.warn("No default reviewers found in CODEOWNERS (or they only contained teams/author)."); } } - // 6. Remove default team if an individual or specific route reviewer was assigned if (assigned) { - await removeDefaultTeamIfRequested(); + console.log("Removing default cloud-sdk-nodejs-team reviewer..."); + try { + await github.rest.pulls.removeRequestedReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + reviewers: [], + team_reviewers: ['cloud-sdk-nodejs-team'], + }); + } catch (err) { + console.warn("Failed to remove default cloud-sdk-nodejs-team reviewer:", err.message || err); + } }