Skip to content

Commit 77bdaa6

Browse files
darshit2308NssGourav
authored andcommitted
feat: automated review queue label sync , Phase 1 (label sync only) (hiero-ledger#2242)
Signed-off-by: darshit2308 <darshit2308@gmail.com>
1 parent c4d535f commit 77bdaa6

11 files changed

Lines changed: 1259 additions & 0 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
//
3+
// helpers/constants.js
4+
//
5+
// Shared constants for the Review Queue Label Sync.
6+
7+
/**
8+
* Safety floor for the GitHub API rate limit.
9+
* The cron job will skip processing if remaining calls fall below this.
10+
*/
11+
const RATE_LIMIT_FLOOR = 200;
12+
13+
/**
14+
* The four mutually exclusive queue labels managed by this bot.
15+
*
16+
* Flow:
17+
* queue:junior-committer → queue:committers → queue:maintainers → status: ready-to-merge
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+
MAINTAINERS: {
31+
name: 'queue:maintainers',
32+
color: 'd876e3',
33+
description: 'PR awaiting maintainer final review',
34+
},
35+
MERGE: {
36+
name: 'status: ready-to-merge',
37+
color: '0e8a16',
38+
description: 'PR has 1+ maintainer and 2+ total approvals, ready to merge',
39+
},
40+
};
41+
42+
/** All queue label names, used for cleanup operations. */
43+
const ALL_QUEUE_LABEL_NAMES = Object.values(QUEUE_LABELS).map((l) => l.name);
44+
45+
module.exports = { RATE_LIMIT_FLOOR, QUEUE_LABELS, ALL_QUEUE_LABEL_NAMES };
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
//
3+
// helpers/index.js
4+
//
5+
// Namespaced barrel export for review-sync helpers.
6+
// Uses namespacing to prevent silent name collisions from spread exports.
7+
//
8+
// Usage:
9+
// const helpers = require('./helpers');
10+
// const { QUEUE_LABELS } = helpers.constants;
11+
// const { syncLabel } = helpers.labels;
12+
13+
module.exports = {
14+
constants: require('./constants'),
15+
permissions: require('./permissions'),
16+
reviews: require('./reviews'),
17+
labels: require('./labels'),
18+
};
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
//
3+
// helpers/labels.js
4+
//
5+
// Label creation, determination, and synchronization.
6+
//
7+
// Key design decisions:
8+
// - ensureLabel() silently handles 422 (race condition / concurrent run)
9+
// - determineLabel() uses a 4-stage pipeline gated by maintainer approval
10+
// - syncLabel() adds the correct label FIRST, then removes stale ones
11+
// (crash-safe: PR never has zero queue labels)
12+
13+
const { QUEUE_LABELS, ALL_QUEUE_LABEL_NAMES } = require('./constants');
14+
const { countApprovals } = require('./permissions');
15+
16+
/**
17+
* Ensure a single label exists in the repo.
18+
* Silently handles 422 (label already exists).
19+
*
20+
* Note: checks existence only. If a label already exists with the wrong
21+
* colour or description, it will not be corrected in Phase 1.
22+
*/
23+
async function ensureLabel(github, owner, repo, label, dryRun) {
24+
try {
25+
await github.rest.issues.getLabel({ owner, repo, name: label.name });
26+
console.log(` Label "${label.name}" already exists. Skipping creation.`);
27+
} catch (error) {
28+
if (error.status === 404) {
29+
if (dryRun) {
30+
console.log(` [DRY RUN] Would create label "${label.name}" (${label.color}).`);
31+
return;
32+
}
33+
try {
34+
await github.rest.issues.createLabel({
35+
owner,
36+
repo,
37+
name: label.name,
38+
color: label.color,
39+
description: label.description,
40+
});
41+
console.log(` Created label "${label.name}" (#${label.color}).`);
42+
} catch (createError) {
43+
// 422 = label already exists (race condition or concurrent run)
44+
if (createError.status === 422) {
45+
console.log(` Label "${label.name}" already exists (422). Skipping.`);
46+
} else {
47+
throw createError;
48+
}
49+
}
50+
} else {
51+
throw error;
52+
}
53+
}
54+
}
55+
56+
/**
57+
* Determine the correct queue label for a PR based on approval counts.
58+
*
59+
* Phase 1 logic (4-stage pipeline):
60+
* maintainerApproval >= 1 AND (maintainerApproval + writeApproval) >= 2 → status: ready-to-merge (CODEOWNERS + min core reviews)
61+
* writeApproval >= 1 OR maintainerApproval >= 1 → queue:maintainers (senior review present, needs more)
62+
* anyApproval >= 1 → queue:committers (has any approval, needs committer)
63+
* else → queue:junior-committer (no approvals yet)
64+
*
65+
* Note: status: ready-to-merge requires BOTH a maintainer approval AND at least 2
66+
* total core reviews (maintainer or write). This prevents a single maintainer approval
67+
* + a soft approval from marking a PR as ready when branch protection requires 2+ core reviews.
68+
*
69+
* @param {{ maintainerApproval: number, writeApproval: number, softApproval: number, anyApproval: number }} approvals
70+
* @returns {object} The correct QUEUE_LABELS entry
71+
*/
72+
function determineLabel(approvals) {
73+
if (approvals.maintainerApproval >= 1 && (approvals.maintainerApproval + approvals.writeApproval) >= 2) {
74+
return QUEUE_LABELS.MERGE;
75+
}
76+
if (approvals.writeApproval >= 1 || approvals.maintainerApproval >= 1) {
77+
return QUEUE_LABELS.MAINTAINERS;
78+
}
79+
if (approvals.anyApproval >= 1) {
80+
return QUEUE_LABELS.COMMITTERS;
81+
}
82+
return QUEUE_LABELS.JUNIOR;
83+
}
84+
85+
/**
86+
* Sync the queue label on a single PR.
87+
*
88+
* Order of operations (non-negotiable):
89+
* 1. Compute stale queue labels on the PR
90+
* 2. Skip only if correct label is already present AND no stale labels exist
91+
* 3. ADD the correct label first
92+
* 4. THEN remove any stale queue labels
93+
*
94+
* This ensures a PR never has zero queue labels, even if the process
95+
* crashes mid-run. Stale labels are always cleaned up, even when the
96+
* correct label was already present.
97+
*
98+
* @param {object} github - Octokit instance
99+
* @param {string} owner - Repository owner
100+
* @param {string} repo - Repository name
101+
* @param {object} pr - Pull request object from the list API
102+
* @param {boolean} dryRun - If true, log without making changes
103+
* @returns {boolean} true if the label was changed, false if already correct
104+
*/
105+
async function syncLabel(github, owner, repo, pr, dryRun) {
106+
const prNumber = pr.number;
107+
const currentLabels = (pr.labels || []).map((l) => l.name);
108+
109+
// Count approvals and determine the correct label
110+
const approvals = await countApprovals(github, owner, repo, prNumber);
111+
const correctLabel = determineLabel(approvals);
112+
113+
console.log(
114+
` PR #${prNumber}: maintainerApproval=${approvals.maintainerApproval}, ` +
115+
`writeApproval=${approvals.writeApproval}, ` +
116+
`softApproval=${approvals.softApproval}, anyApproval=${approvals.anyApproval} ` +
117+
`→ ${correctLabel.name}`
118+
);
119+
120+
// Determine which stale queue labels to remove
121+
const staleLabels = currentLabels.filter(
122+
(name) => ALL_QUEUE_LABEL_NAMES.includes(name) && name !== correctLabel.name
123+
);
124+
125+
// Check if the correct label is already present AND there are no stale labels to remove
126+
if (currentLabels.includes(correctLabel.name) && staleLabels.length === 0) {
127+
console.log(` ✓ Already has "${correctLabel.name}". No change needed.`);
128+
return false;
129+
}
130+
131+
if (dryRun) {
132+
console.log(` [DRY RUN] Would add "${correctLabel.name}".`);
133+
if (staleLabels.length > 0) {
134+
console.log(` [DRY RUN] Would remove: ${staleLabels.join(', ')}.`);
135+
}
136+
return true;
137+
}
138+
139+
// Step 1: ADD the correct label FIRST (crash-safe: PR always has at least one label)
140+
await github.rest.issues.addLabels({
141+
owner,
142+
repo,
143+
issue_number: prNumber,
144+
labels: [correctLabel.name],
145+
});
146+
console.log(` + Added "${correctLabel.name}".`);
147+
148+
// Step 2: THEN remove stale queue labels one by one
149+
for (const stale of staleLabels) {
150+
try {
151+
await github.rest.issues.removeLabel({
152+
owner,
153+
repo,
154+
issue_number: prNumber,
155+
name: stale,
156+
});
157+
console.log(` - Removed "${stale}".`);
158+
} catch (error) {
159+
// 404 = label was already removed (race condition or manual action)
160+
if (error.status === 404) {
161+
console.log(` - Label "${stale}" already gone (404). Skipping.`);
162+
} else {
163+
const message = error instanceof Error ? error.message : String(error);
164+
console.error(` ✗ Failed to remove "${stale}": ${message}`);
165+
throw error; // Re-throw to prevent silently leaving PR in a broken multi-label state
166+
}
167+
}
168+
}
169+
170+
return true;
171+
}
172+
173+
module.exports = { ensureLabel, determineLabel, syncLabel };
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
//
3+
// helpers/permissions.js
4+
//
5+
// Permission level checks and approval counting.
6+
//
7+
// CRITICAL NOTE ON role_name vs permission:
8+
// getCollaboratorPermissionLevel returns TWO fields:
9+
// - permission (legacy): maps maintain → write, triage → read
10+
// - role_name (accurate): returns admin | maintain | write | triage | read
11+
//
12+
// We MUST use role_name to distinguish maintainers from committers.
13+
// If we used permission, Sophie (Maintain role per MAINTAINERS.md) would
14+
// appear as 'write', and maintainerApproval would always be 0.
15+
// PRs would be permanently stuck at queue:maintainers.
16+
//
17+
// Phase 3 will replace this with team membership checks
18+
// (getMembershipForUserInOrg) for full accuracy.
19+
20+
const { getLatestReviewStates } = require('./reviews');
21+
22+
/**
23+
* Check the repository role for a given user.
24+
*
25+
* Uses role_name (not legacy permission) to correctly detect the maintain role.
26+
*
27+
* @param {object} github - Octokit instance
28+
* @param {string} owner - Repository owner
29+
* @param {string} repo - Repository name
30+
* @param {string} username - GitHub username
31+
* @returns {string} 'admin' | 'maintain' | 'write' | 'triage' | 'read' | 'none'
32+
*/
33+
async function getPermissionLevel(github, owner, repo, username) {
34+
try {
35+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
36+
owner,
37+
repo,
38+
username,
39+
});
40+
// CRITICAL: Use role_name, NOT permission.
41+
// The legacy 'permission' field maps maintain → write, triage → read.
42+
// role_name correctly returns: admin | maintain | write | triage | read
43+
return data.role_name || data.permission || 'none';
44+
} catch (error) {
45+
if (error.status === 404) {
46+
// External contributor — not a collaborator
47+
return 'none';
48+
}
49+
// Log unexpected errors but don't crash the run
50+
const message = error instanceof Error ? error.message : String(error);
51+
console.log(` ⚠ Permission check failed for ${username}: ${message}. Treating as "none".`);
52+
return 'none';
53+
}
54+
}
55+
56+
/**
57+
* Count approvals on a PR, split by permission level.
58+
*
59+
* Returns three counters:
60+
* - maintainerApproval: admin or maintain (maps to CODEOWNERS maintainer teams)
61+
* - writeApproval: write (committers)
62+
* - softApproval: triage, read, none, external contributors
63+
*
64+
* @param {object} github - Octokit instance
65+
* @param {string} owner - Repository owner
66+
* @param {string} repo - Repository name
67+
* @param {number} prNumber - Pull request number
68+
* @returns {{ maintainerApproval: number, writeApproval: number, softApproval: number, anyApproval: number }}
69+
*/
70+
async function countApprovals(github, owner, repo, prNumber) {
71+
const latestStates = await getLatestReviewStates(github, owner, repo, prNumber);
72+
73+
let maintainerApproval = 0;
74+
let writeApproval = 0;
75+
let softApproval = 0;
76+
77+
for (const [username, state] of latestStates) {
78+
if (state !== 'APPROVED') continue;
79+
80+
const role = await getPermissionLevel(github, owner, repo, username);
81+
82+
if (role === 'admin' || role === 'maintain') {
83+
maintainerApproval++;
84+
} else if (role === 'write') {
85+
writeApproval++;
86+
} else {
87+
// triage, read, none, or any unexpected value → soft approval
88+
softApproval++;
89+
}
90+
}
91+
92+
return {
93+
maintainerApproval,
94+
writeApproval,
95+
softApproval,
96+
anyApproval: maintainerApproval + writeApproval + softApproval,
97+
};
98+
}
99+
100+
module.exports = { getPermissionLevel, countApprovals };

0 commit comments

Comments
 (0)