Skip to content

Commit 04f013c

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 - Rate-limit guard (floor=200) prevents partial runs - 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 04f013c

3 files changed

Lines changed: 434 additions & 0 deletions

File tree

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

0 commit comments

Comments
 (0)