Skip to content

Commit e79b9e5

Browse files
committed
feat: add Phase 1 review queue label sync
Introduce a cron-based workflow (every 30 min) that classifies every open non-draft PR into one of three review stages based on the number and permission level of approvals: queue:junior-committer → queue:committers → ready-to-merge Phase 1 of 4 — label sync only. No assignments, comments, or routing. Label determination: - writeApproval >= 2 → ready-to-merge - anyApproval >= 1 → queue:committers - else → queue:junior-committer Key design decisions: - Add-first-then-remove label ordering prevents zero-label state - DISMISSED reviews actively delete prior state (prevents ghost approvals) - Reviews explicitly sorted by submitted_at (correct by construction) - Rate-limit guard (floor=200) prevents partial runs - Concurrency block prevents overlapping cron runs from racing - DRY_RUN mode for safe manual testing via workflow_dispatch - 422/404 errors handled silently (race conditions, external users) - Old step:/reviewer: labels coexist intentionally until future cleanup - Bot-authored PRs (Dependabot) receive labels (acknowledged) Files added: .github/workflows/review-sync.yml .github/scripts/review-sync/index.js .github/scripts/review-sync/labels.js Signed-off-by: darshit2308 <darshitpatel2003@gmail.com>
1 parent 79b34a7 commit e79b9e5

3 files changed

Lines changed: 451 additions & 0 deletions

File tree

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// .github/scripts/review-sync/index.js
2+
//
3+
// Entry point for the Review Queue Label Sync cron job.
4+
//
5+
// Responsibilities:
6+
// 1. Rate-limit guard — abort if remaining calls < 200
7+
// 2. Fetch all open non-draft PRs (paginated)
8+
// 3. Ensure the three queue labels exist in the repo
9+
// 4. Sync the correct label on every PR via labels.js
10+
// 5. Print a summary of what changed
11+
//
12+
// Phase 1 of 4 — label sync only.
13+
14+
const { QUEUE_LABELS, syncLabel } = require('./labels.js');
15+
16+
const RATE_LIMIT_FLOOR = 200;
17+
18+
/**
19+
* Ensure a single label exists in the repo.
20+
* Silently handles 422 (label already exists).
21+
*
22+
* Note: checks existence only. If a label already exists with the wrong
23+
* colour or description, it will not be corrected in Phase 1.
24+
*/
25+
async function ensureLabel(github, owner, repo, label, dryRun) {
26+
try {
27+
await github.rest.issues.getLabel({ owner, repo, name: label.name });
28+
console.log(` Label "${label.name}" already exists. Skipping creation.`);
29+
} catch (error) {
30+
if (error.status === 404) {
31+
if (dryRun) {
32+
console.log(` [DRY RUN] Would create label "${label.name}" (${label.color}).`);
33+
return;
34+
}
35+
try {
36+
await github.rest.issues.createLabel({
37+
owner,
38+
repo,
39+
name: label.name,
40+
color: label.color,
41+
description: label.description,
42+
});
43+
console.log(` Created label "${label.name}" (#${label.color}).`);
44+
} catch (createError) {
45+
// 422 = label already exists (race condition or concurrent run)
46+
if (createError.status === 422) {
47+
console.log(` Label "${label.name}" already exists (422). Skipping.`);
48+
} else {
49+
throw createError;
50+
}
51+
}
52+
} else {
53+
throw error;
54+
}
55+
}
56+
}
57+
58+
module.exports = async ({ github, context, core }) => {
59+
const dryRun = (process.env.DRY_RUN || 'false').toLowerCase() === 'true';
60+
const { owner, repo } = context.repo;
61+
62+
if (dryRun) {
63+
console.log('=== DRY RUN MODE — no labels will be created or modified ===\n');
64+
}
65+
66+
// ── 1. Rate-limit guard ──────────────────────────────────────────────────
67+
console.log('--- Rate Limit Check ---');
68+
const { data: rateLimit } = await github.rest.rateLimit.get();
69+
const remaining = rateLimit.resources.core.remaining;
70+
console.log(` Core API remaining: ${remaining}`);
71+
72+
if (remaining < RATE_LIMIT_FLOOR) {
73+
console.log(` ⚠ Skipping run: rate limit too low (${remaining} < ${RATE_LIMIT_FLOOR}).`);
74+
return;
75+
}
76+
77+
// ── 2. Fetch all open non-draft PRs (paginated) ──────────────────────────
78+
console.log('\n--- Fetching Open PRs ---');
79+
const allPRs = await github.paginate(github.rest.pulls.list, {
80+
owner,
81+
repo,
82+
state: 'open',
83+
per_page: 100,
84+
});
85+
86+
const prs = allPRs.filter((pr) => !pr.draft);
87+
console.log(` Total open PRs: ${allPRs.length}`);
88+
console.log(` Non-draft PRs to process: ${prs.length}`);
89+
console.log(` Draft PRs skipped: ${allPRs.length - prs.length}`);
90+
91+
if (prs.length === 0) {
92+
console.log(' No non-draft PRs found. Exiting.');
93+
return;
94+
}
95+
96+
// ── 3. Ensure queue labels exist ─────────────────────────────────────────
97+
console.log('\n--- Ensuring Queue Labels Exist ---');
98+
for (const label of Object.values(QUEUE_LABELS)) {
99+
await ensureLabel(github, owner, repo, label, dryRun);
100+
}
101+
102+
// ── 4. Sync label on each PR ─────────────────────────────────────────────
103+
console.log('\n--- Syncing Labels ---');
104+
let changed = 0;
105+
let skipped = 0;
106+
let errors = 0;
107+
108+
for (const pr of prs) {
109+
try {
110+
const didChange = await syncLabel(github, owner, repo, pr, dryRun);
111+
if (didChange) {
112+
changed++;
113+
} else {
114+
skipped++;
115+
}
116+
} catch (error) {
117+
errors++;
118+
const message = error instanceof Error ? error.message : String(error);
119+
console.error(` ✗ Error on PR #${pr.number}: ${message}`);
120+
}
121+
}
122+
123+
// ── 5. Summary ───────────────────────────────────────────────────────────
124+
console.log('\n=== Summary ===');
125+
console.log(` PRs processed: ${prs.length}`);
126+
console.log(` Labels changed: ${changed}`);
127+
console.log(` Labels already correct: ${skipped}`);
128+
console.log(` Errors: ${errors}`);
129+
};
Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
// .github/scripts/review-sync/labels.js
2+
//
3+
// Core label determination and application for the Review Queue Sync.
4+
//
5+
// Logic (Phase 1 — simplified, no difficulty routing):
6+
// 1. Fetch all reviews on a PR (paginated, sorted by submitted_at)
7+
// 2. Build username → latest review state map
8+
// 3. Ignore COMMENTED; DISMISSED actively deletes prior state (prevents ghost approvals)
9+
// 4. For each APPROVED reviewer, check permission level
10+
// 5. Determine correct queue label based on approval counts
11+
// 6. Apply label: add new FIRST, then remove stale (non-negotiable order)
12+
//
13+
// Phase 2 will add difficulty-based routing on top of this logic.
14+
15+
/**
16+
* The three mutually exclusive queue labels managed by this bot.
17+
* Colours and names match the Phase 1 specification.
18+
*/
19+
const QUEUE_LABELS = {
20+
JUNIOR: {
21+
name: 'queue:junior-committer',
22+
color: 'e4e669',
23+
description: 'PR awaiting initial quality review',
24+
},
25+
COMMITTERS: {
26+
name: 'queue:committers',
27+
color: '0075ca',
28+
description: 'PR awaiting committer technical review',
29+
},
30+
MERGE: {
31+
name: 'ready-to-merge',
32+
color: '0e8a16',
33+
description: 'PR has 2+ write approvals and is ready to merge',
34+
},
35+
};
36+
37+
/** All queue label names, used for cleanup operations. */
38+
const ALL_QUEUE_LABEL_NAMES = Object.values(QUEUE_LABELS).map((l) => l.name);
39+
40+
/**
41+
* Fetch all reviews on a PR, returning only the latest state per reviewer.
42+
* COMMENTED reviews are ignored. DISMISSED reviews actively delete prior state
43+
* to prevent ghost approvals (e.g. when GitHub auto-dismisses stale reviews
44+
* after new commits are pushed).
45+
*
46+
* @param {object} github - Octokit instance
47+
* @param {string} owner - Repository owner
48+
* @param {string} repo - Repository name
49+
* @param {number} prNumber - Pull request number
50+
* @returns {Map<string, string>} username → latest review state (APPROVED | CHANGES_REQUESTED)
51+
*/
52+
async function getLatestReviewStates(github, owner, repo, prNumber) {
53+
const reviews = await github.paginate(github.rest.pulls.listReviews, {
54+
owner,
55+
repo,
56+
pull_number: prNumber,
57+
per_page: 100,
58+
});
59+
60+
// Sort explicitly by submitted_at to guarantee chronological order.
61+
// The GitHub API returns reviews chronologically in practice, but
62+
// explicit sorting makes this correct by construction.
63+
const sortedReviews = [...reviews].sort(
64+
(a, b) => new Date(a.submitted_at) - new Date(b.submitted_at)
65+
);
66+
67+
// Build a map keyed by reviewer login.
68+
// Later entries overwrite earlier ones — giving us the latest state per user.
69+
const latestByUser = new Map();
70+
71+
for (const review of sortedReviews) {
72+
const login = review.user?.login;
73+
const state = review.state?.toUpperCase();
74+
75+
if (!login || !state) continue;
76+
77+
if (state === 'APPROVED' || state === 'CHANGES_REQUESTED') {
78+
latestByUser.set(login, state);
79+
} else if (state === 'DISMISSED') {
80+
// CRITICAL: A dismissed review wipes out the user's prior approval.
81+
// Without this, a stale review that GitHub auto-dismissed (e.g. after
82+
// new commits) would persist as a ghost approval in the map.
83+
latestByUser.delete(login);
84+
}
85+
// COMMENTED is the only state intentionally ignored
86+
}
87+
88+
return latestByUser;
89+
}
90+
91+
/**
92+
* Check the repository permission level for a given user.
93+
*
94+
* @param {object} github - Octokit instance
95+
* @param {string} owner - Repository owner
96+
* @param {string} repo - Repository name
97+
* @param {string} username - GitHub username
98+
* @returns {string} 'admin' | 'write' | 'read' | 'none'
99+
*/
100+
async function getPermissionLevel(github, owner, repo, username) {
101+
try {
102+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
103+
owner,
104+
repo,
105+
username,
106+
});
107+
return data.permission || 'none';
108+
} catch (error) {
109+
if (error.status === 404) {
110+
// External contributor — not a collaborator
111+
return 'none';
112+
}
113+
// Log unexpected errors but don't crash the run
114+
const message = error instanceof Error ? error.message : String(error);
115+
console.log(` ⚠ Permission check failed for ${username}: ${message}. Treating as "none".`);
116+
return 'none';
117+
}
118+
}
119+
120+
/**
121+
* Count approvals on a PR, split by permission level.
122+
*
123+
* @param {object} github - Octokit instance
124+
* @param {string} owner - Repository owner
125+
* @param {string} repo - Repository name
126+
* @param {number} prNumber - Pull request number
127+
* @returns {{ writeApproval: number, softApproval: number, anyApproval: number }}
128+
*/
129+
async function countApprovals(github, owner, repo, prNumber) {
130+
const latestStates = await getLatestReviewStates(github, owner, repo, prNumber);
131+
132+
let writeApproval = 0;
133+
let softApproval = 0;
134+
135+
for (const [username, state] of latestStates) {
136+
if (state !== 'APPROVED') continue;
137+
138+
const permission = await getPermissionLevel(github, owner, repo, username);
139+
140+
if (permission === 'admin' || permission === 'write') {
141+
writeApproval++;
142+
} else {
143+
// read, none, or any unexpected value → soft approval
144+
softApproval++;
145+
}
146+
}
147+
148+
return {
149+
writeApproval,
150+
softApproval,
151+
anyApproval: writeApproval + softApproval,
152+
};
153+
}
154+
155+
/**
156+
* Determine the correct queue label for a PR based on approval counts.
157+
*
158+
* Phase 1 logic (no difficulty routing):
159+
* writeApproval >= 2 → ready-to-merge
160+
* anyApproval >= 1 → queue:committers
161+
* else → queue:junior-committer
162+
*
163+
* @param {{ writeApproval: number, anyApproval: number }} approvals
164+
* @returns {object} The correct QUEUE_LABELS entry
165+
*/
166+
function determineLabel(approvals) {
167+
if (approvals.writeApproval >= 2) {
168+
return QUEUE_LABELS.MERGE;
169+
}
170+
if (approvals.anyApproval >= 1) {
171+
return QUEUE_LABELS.COMMITTERS;
172+
}
173+
return QUEUE_LABELS.JUNIOR;
174+
}
175+
176+
/**
177+
* Sync the queue label on a single PR.
178+
*
179+
* Order of operations (non-negotiable):
180+
* 1. Check if the correct label is already present → skip if yes
181+
* 2. ADD the correct label first
182+
* 3. THEN remove any stale queue labels
183+
*
184+
* This ensures a PR never has zero queue labels, even if the process
185+
* crashes mid-run.
186+
*
187+
* @param {object} github - Octokit instance
188+
* @param {string} owner - Repository owner
189+
* @param {string} repo - Repository name
190+
* @param {object} pr - Pull request object from the list API
191+
* @param {boolean} dryRun - If true, log without making changes
192+
* @returns {boolean} true if the label was changed, false if already correct
193+
*/
194+
async function syncLabel(github, owner, repo, pr, dryRun) {
195+
const prNumber = pr.number;
196+
const currentLabels = (pr.labels || []).map((l) => l.name);
197+
198+
// Count approvals and determine the correct label
199+
const approvals = await countApprovals(github, owner, repo, prNumber);
200+
const correctLabel = determineLabel(approvals);
201+
202+
console.log(
203+
` PR #${prNumber}: writeApproval=${approvals.writeApproval}, ` +
204+
`softApproval=${approvals.softApproval}, anyApproval=${approvals.anyApproval} ` +
205+
`→ ${correctLabel.name}`
206+
);
207+
208+
// Check if the correct label is already present
209+
if (currentLabels.includes(correctLabel.name)) {
210+
console.log(` ✓ Already has "${correctLabel.name}". No change needed.`);
211+
return false;
212+
}
213+
214+
// Determine which stale queue labels to remove
215+
const staleLabels = currentLabels.filter(
216+
(name) => ALL_QUEUE_LABEL_NAMES.includes(name) && name !== correctLabel.name
217+
);
218+
219+
if (dryRun) {
220+
console.log(` [DRY RUN] Would add "${correctLabel.name}".`);
221+
if (staleLabels.length > 0) {
222+
console.log(` [DRY RUN] Would remove: ${staleLabels.join(', ')}.`);
223+
}
224+
return true;
225+
}
226+
227+
// Step 1: ADD the correct label FIRST (crash-safe: PR always has at least one label)
228+
await github.rest.issues.addLabels({
229+
owner,
230+
repo,
231+
issue_number: prNumber,
232+
labels: [correctLabel.name],
233+
});
234+
console.log(` + Added "${correctLabel.name}".`);
235+
236+
// Step 2: THEN remove stale queue labels one by one
237+
for (const stale of staleLabels) {
238+
try {
239+
await github.rest.issues.removeLabel({
240+
owner,
241+
repo,
242+
issue_number: prNumber,
243+
name: stale,
244+
});
245+
console.log(` - Removed "${stale}".`);
246+
} catch (error) {
247+
// 404 = label was already removed (race condition or manual action)
248+
if (error.status === 404) {
249+
console.log(` - Label "${stale}" already gone (404). Skipping.`);
250+
} else {
251+
const message = error instanceof Error ? error.message : String(error);
252+
console.error(` ✗ Failed to remove "${stale}": ${message}`);
253+
}
254+
}
255+
}
256+
257+
return true;
258+
}
259+
260+
module.exports = {
261+
QUEUE_LABELS,
262+
ALL_QUEUE_LABEL_NAMES,
263+
syncLabel,
264+
};

0 commit comments

Comments
 (0)