diff --git a/.github/scripts/review-sync/helpers/constants.js b/.github/scripts/review-sync/helpers/constants.js new file mode 100644 index 000000000..f23dcf3f4 --- /dev/null +++ b/.github/scripts/review-sync/helpers/constants.js @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// helpers/constants.js +// +// Shared constants for the Review Queue Label Sync. + +/** + * Safety floor for the GitHub API rate limit. + * The cron job will skip processing if remaining calls fall below this. + */ +const RATE_LIMIT_FLOOR = 200; + +/** + * The four mutually exclusive queue labels managed by this bot. + * + * Flow: + * queue:junior-committer → queue:committers → queue:maintainers → status: ready-to-merge + */ +const QUEUE_LABELS = { + JUNIOR: { + name: 'queue:junior-committer', + color: 'e4e669', + description: 'PR awaiting initial quality review', + }, + COMMITTERS: { + name: 'queue:committers', + color: '0075ca', + description: 'PR awaiting committer technical review', + }, + MAINTAINERS: { + name: 'queue:maintainers', + color: 'd876e3', + description: 'PR awaiting maintainer final review', + }, + MERGE: { + name: 'status: ready-to-merge', + color: '0e8a16', + description: 'PR has 1+ maintainer and 2+ total approvals, ready to merge', + }, +}; + +/** All queue label names, used for cleanup operations. */ +const ALL_QUEUE_LABEL_NAMES = Object.values(QUEUE_LABELS).map((l) => l.name); + +module.exports = { RATE_LIMIT_FLOOR, QUEUE_LABELS, ALL_QUEUE_LABEL_NAMES }; diff --git a/.github/scripts/review-sync/helpers/index.js b/.github/scripts/review-sync/helpers/index.js new file mode 100644 index 000000000..22ef9ee28 --- /dev/null +++ b/.github/scripts/review-sync/helpers/index.js @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// helpers/index.js +// +// Namespaced barrel export for review-sync helpers. +// Uses namespacing to prevent silent name collisions from spread exports. +// +// Usage: +// const helpers = require('./helpers'); +// const { QUEUE_LABELS } = helpers.constants; +// const { syncLabel } = helpers.labels; + +module.exports = { + constants: require('./constants'), + permissions: require('./permissions'), + reviews: require('./reviews'), + labels: require('./labels'), +}; diff --git a/.github/scripts/review-sync/helpers/labels.js b/.github/scripts/review-sync/helpers/labels.js new file mode 100644 index 000000000..596cebbeb --- /dev/null +++ b/.github/scripts/review-sync/helpers/labels.js @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// helpers/labels.js +// +// Label creation, determination, and synchronization. +// +// Key design decisions: +// - ensureLabel() silently handles 422 (race condition / concurrent run) +// - determineLabel() uses a 4-stage pipeline gated by maintainer approval +// - syncLabel() adds the correct label FIRST, then removes stale ones +// (crash-safe: PR never has zero queue labels) + +const { QUEUE_LABELS, ALL_QUEUE_LABEL_NAMES } = require('./constants'); +const { countApprovals } = require('./permissions'); + +/** + * Ensure a single label exists in the repo. + * Silently handles 422 (label already exists). + * + * Note: checks existence only. If a label already exists with the wrong + * colour or description, it will not be corrected in Phase 1. + */ +async function ensureLabel(github, owner, repo, label, dryRun) { + try { + await github.rest.issues.getLabel({ owner, repo, name: label.name }); + console.log(` Label "${label.name}" already exists. Skipping creation.`); + } catch (error) { + if (error.status === 404) { + if (dryRun) { + console.log(` [DRY RUN] Would create label "${label.name}" (${label.color}).`); + return; + } + try { + await github.rest.issues.createLabel({ + owner, + repo, + name: label.name, + color: label.color, + description: label.description, + }); + console.log(` Created label "${label.name}" (#${label.color}).`); + } catch (createError) { + // 422 = label already exists (race condition or concurrent run) + if (createError.status === 422) { + console.log(` Label "${label.name}" already exists (422). Skipping.`); + } else { + throw createError; + } + } + } else { + throw error; + } + } +} + +/** + * Determine the correct queue label for a PR based on approval counts. + * + * Phase 1 logic (4-stage pipeline): + * maintainerApproval >= 1 AND (maintainerApproval + writeApproval) >= 2 → status: ready-to-merge (CODEOWNERS + min core reviews) + * writeApproval >= 1 OR maintainerApproval >= 1 → queue:maintainers (senior review present, needs more) + * anyApproval >= 1 → queue:committers (has any approval, needs committer) + * else → queue:junior-committer (no approvals yet) + * + * Note: status: ready-to-merge requires BOTH a maintainer approval AND at least 2 + * total core reviews (maintainer or write). This prevents a single maintainer approval + * + a soft approval from marking a PR as ready when branch protection requires 2+ core reviews. + * + * @param {{ maintainerApproval: number, writeApproval: number, softApproval: number, anyApproval: number }} approvals + * @returns {object} The correct QUEUE_LABELS entry + */ +function determineLabel(approvals) { + if (approvals.maintainerApproval >= 1 && (approvals.maintainerApproval + approvals.writeApproval) >= 2) { + return QUEUE_LABELS.MERGE; + } + if (approvals.writeApproval >= 1 || approvals.maintainerApproval >= 1) { + return QUEUE_LABELS.MAINTAINERS; + } + if (approvals.anyApproval >= 1) { + return QUEUE_LABELS.COMMITTERS; + } + return QUEUE_LABELS.JUNIOR; +} + +/** + * Sync the queue label on a single PR. + * + * Order of operations (non-negotiable): + * 1. Compute stale queue labels on the PR + * 2. Skip only if correct label is already present AND no stale labels exist + * 3. ADD the correct label first + * 4. THEN remove any stale queue labels + * + * This ensures a PR never has zero queue labels, even if the process + * crashes mid-run. Stale labels are always cleaned up, even when the + * correct label was already present. + * + * @param {object} github - Octokit instance + * @param {string} owner - Repository owner + * @param {string} repo - Repository name + * @param {object} pr - Pull request object from the list API + * @param {boolean} dryRun - If true, log without making changes + * @returns {boolean} true if the label was changed, false if already correct + */ +async function syncLabel(github, owner, repo, pr, dryRun) { + const prNumber = pr.number; + const currentLabels = (pr.labels || []).map((l) => l.name); + + // Count approvals and determine the correct label + const approvals = await countApprovals(github, owner, repo, prNumber); + const correctLabel = determineLabel(approvals); + + console.log( + ` PR #${prNumber}: maintainerApproval=${approvals.maintainerApproval}, ` + + `writeApproval=${approvals.writeApproval}, ` + + `softApproval=${approvals.softApproval}, anyApproval=${approvals.anyApproval} ` + + `→ ${correctLabel.name}` + ); + + // Determine which stale queue labels to remove + const staleLabels = currentLabels.filter( + (name) => ALL_QUEUE_LABEL_NAMES.includes(name) && name !== correctLabel.name + ); + + // Check if the correct label is already present AND there are no stale labels to remove + if (currentLabels.includes(correctLabel.name) && staleLabels.length === 0) { + console.log(` ✓ Already has "${correctLabel.name}". No change needed.`); + return false; + } + + if (dryRun) { + console.log(` [DRY RUN] Would add "${correctLabel.name}".`); + if (staleLabels.length > 0) { + console.log(` [DRY RUN] Would remove: ${staleLabels.join(', ')}.`); + } + return true; + } + + // Step 1: ADD the correct label FIRST (crash-safe: PR always has at least one label) + await github.rest.issues.addLabels({ + owner, + repo, + issue_number: prNumber, + labels: [correctLabel.name], + }); + console.log(` + Added "${correctLabel.name}".`); + + // Step 2: THEN remove stale queue labels one by one + for (const stale of staleLabels) { + try { + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: prNumber, + name: stale, + }); + console.log(` - Removed "${stale}".`); + } catch (error) { + // 404 = label was already removed (race condition or manual action) + if (error.status === 404) { + console.log(` - Label "${stale}" already gone (404). Skipping.`); + } else { + const message = error instanceof Error ? error.message : String(error); + console.error(` ✗ Failed to remove "${stale}": ${message}`); + throw error; // Re-throw to prevent silently leaving PR in a broken multi-label state + } + } + } + + return true; +} + +module.exports = { ensureLabel, determineLabel, syncLabel }; diff --git a/.github/scripts/review-sync/helpers/permissions.js b/.github/scripts/review-sync/helpers/permissions.js new file mode 100644 index 000000000..fd3dd970b --- /dev/null +++ b/.github/scripts/review-sync/helpers/permissions.js @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// helpers/permissions.js +// +// Permission level checks and approval counting. +// +// CRITICAL NOTE ON role_name vs permission: +// getCollaboratorPermissionLevel returns TWO fields: +// - permission (legacy): maps maintain → write, triage → read +// - role_name (accurate): returns admin | maintain | write | triage | read +// +// We MUST use role_name to distinguish maintainers from committers. +// If we used permission, Sophie (Maintain role per MAINTAINERS.md) would +// appear as 'write', and maintainerApproval would always be 0. +// PRs would be permanently stuck at queue:maintainers. +// +// Phase 3 will replace this with team membership checks +// (getMembershipForUserInOrg) for full accuracy. + +const { getLatestReviewStates } = require('./reviews'); + +/** + * Check the repository role for a given user. + * + * Uses role_name (not legacy permission) to correctly detect the maintain role. + * + * @param {object} github - Octokit instance + * @param {string} owner - Repository owner + * @param {string} repo - Repository name + * @param {string} username - GitHub username + * @returns {string} 'admin' | 'maintain' | 'write' | 'triage' | 'read' | 'none' + */ +async function getPermissionLevel(github, owner, repo, username) { + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner, + repo, + username, + }); + // CRITICAL: Use role_name, NOT permission. + // The legacy 'permission' field maps maintain → write, triage → read. + // role_name correctly returns: admin | maintain | write | triage | read + return data.role_name || data.permission || 'none'; + } catch (error) { + if (error.status === 404) { + // External contributor — not a collaborator + return 'none'; + } + // Log unexpected errors but don't crash the run + const message = error instanceof Error ? error.message : String(error); + console.log(` ⚠ Permission check failed for ${username}: ${message}. Treating as "none".`); + return 'none'; + } +} + +/** + * Count approvals on a PR, split by permission level. + * + * Returns three counters: + * - maintainerApproval: admin or maintain (maps to CODEOWNERS maintainer teams) + * - writeApproval: write (committers) + * - softApproval: triage, read, none, external contributors + * + * @param {object} github - Octokit instance + * @param {string} owner - Repository owner + * @param {string} repo - Repository name + * @param {number} prNumber - Pull request number + * @returns {{ maintainerApproval: number, writeApproval: number, softApproval: number, anyApproval: number }} + */ +async function countApprovals(github, owner, repo, prNumber) { + const latestStates = await getLatestReviewStates(github, owner, repo, prNumber); + + let maintainerApproval = 0; + let writeApproval = 0; + let softApproval = 0; + + for (const [username, state] of latestStates) { + if (state !== 'APPROVED') continue; + + const role = await getPermissionLevel(github, owner, repo, username); + + if (role === 'admin' || role === 'maintain') { + maintainerApproval++; + } else if (role === 'write') { + writeApproval++; + } else { + // triage, read, none, or any unexpected value → soft approval + softApproval++; + } + } + + return { + maintainerApproval, + writeApproval, + softApproval, + anyApproval: maintainerApproval + writeApproval + softApproval, + }; +} + +module.exports = { getPermissionLevel, countApprovals }; diff --git a/.github/scripts/review-sync/helpers/reviews.js b/.github/scripts/review-sync/helpers/reviews.js new file mode 100644 index 000000000..670a8ed91 --- /dev/null +++ b/.github/scripts/review-sync/helpers/reviews.js @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// helpers/reviews.js +// +// Fetches and processes PR review states. +// +// Key design decisions: +// - COMMENTED reviews are intentionally ignored (they carry no approval weight) +// - DISMISSED reviews actively delete prior state to prevent ghost approvals +// - Explicit chronological sort guarantees correctness regardless of API order + +/** + * Fetch all reviews on a PR, returning only the latest state per reviewer. + * COMMENTED reviews are ignored. DISMISSED reviews actively delete prior state + * to prevent ghost approvals (e.g. when GitHub auto-dismisses stale reviews + * after new commits are pushed). + * + * @param {object} github - Octokit instance + * @param {string} owner - Repository owner + * @param {string} repo - Repository name + * @param {number} prNumber - Pull request number + * @returns {Map} username → latest review state (APPROVED | CHANGES_REQUESTED) + */ +async function getLatestReviewStates(github, owner, repo, prNumber) { + const reviews = await github.paginate(github.rest.pulls.listReviews, { + owner, + repo, + pull_number: prNumber, + per_page: 100, + }); + + // Sort explicitly by submitted_at to guarantee chronological order. + // The GitHub API returns reviews chronologically in practice, but + // explicit sorting makes this correct by construction. + const sortedReviews = [...reviews].sort( + (a, b) => new Date(a.submitted_at) - new Date(b.submitted_at) + ); + + // Build a map keyed by reviewer login. + // Later entries overwrite earlier ones — giving us the latest state per user. + const latestByUser = new Map(); + + for (const review of sortedReviews) { + const login = review.user?.login; + const state = review.state?.toUpperCase(); + + if (!login || !state) continue; + + if (state === 'APPROVED' || state === 'CHANGES_REQUESTED') { + latestByUser.set(login, state); + } else if (state === 'DISMISSED') { + // CRITICAL: A dismissed review wipes out the user's prior approval. + // Without this, a stale review that GitHub auto-dismissed (e.g. after + // new commits) would persist as a ghost approval in the map. + latestByUser.delete(login); + } + // COMMENTED is the only state intentionally ignored + } + + return latestByUser; +} + +module.exports = { getLatestReviewStates }; diff --git a/.github/scripts/review-sync/index.js b/.github/scripts/review-sync/index.js new file mode 100644 index 000000000..5fc13a34c --- /dev/null +++ b/.github/scripts/review-sync/index.js @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// .github/scripts/review-sync/index.js +// +// Entry point for the Review Queue Label Sync cron job. +// +// Responsibilities: +// 1. Rate-limit guard — abort if remaining calls < 200 +// 2. Fetch all open non-draft PRs (paginated) +// 3. Ensure the four queue labels exist in the repo +// 4. Sync the correct label on every PR via helpers +// 5. Print a summary of what changed +// +// Phase 1 of 4 — label sync only. + +const helpers = require('./helpers'); +const { QUEUE_LABELS, RATE_LIMIT_FLOOR } = helpers.constants; +const { ensureLabel, syncLabel } = helpers.labels; + +module.exports = async ({ github, context, core }) => { + const dryRun = (process.env.DRY_RUN || 'false').toLowerCase() === 'true'; + const { owner, repo } = context.repo; + + if (dryRun) { + console.log('=== DRY RUN MODE — no labels will be created or modified ===\n'); + } + + // ── 1. Rate-limit guard ────────────────────────────────────────────────── + console.log('--- Rate Limit Check ---'); + const { data: rateLimit } = await github.rest.rateLimit.get(); + const remaining = rateLimit.resources.core.remaining; + console.log(` Core API remaining: ${remaining}`); + + if (remaining < RATE_LIMIT_FLOOR) { + console.log(` ⚠ Skipping run: rate limit too low (${remaining} < ${RATE_LIMIT_FLOOR}).`); + return; + } + + // ── 2. Fetch all open non-draft PRs (paginated) ────────────────────────── + console.log('\n--- Fetching Open PRs ---'); + const allPRs = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + per_page: 100, + }); + + const prs = allPRs.filter((pr) => !pr.draft); + console.log(` Total open PRs: ${allPRs.length}`); + console.log(` Non-draft PRs to process: ${prs.length}`); + console.log(` Draft PRs skipped: ${allPRs.length - prs.length}`); + + if (prs.length === 0) { + console.log(' No non-draft PRs found. Exiting.'); + return; + } + + // ── 3. Ensure queue labels exist ───────────────────────────────────────── + console.log('\n--- Ensuring Queue Labels Exist ---'); + for (const label of Object.values(QUEUE_LABELS)) { + await ensureLabel(github, owner, repo, label, dryRun); + } + + // ── 4. Sync label on each PR ───────────────────────────────────────────── + console.log('\n--- Syncing Labels ---'); + let changed = 0; + let skipped = 0; + let errors = 0; + + for (const pr of prs) { + try { + const didChange = await syncLabel(github, owner, repo, pr, dryRun); + if (didChange) { + changed++; + } else { + skipped++; + } + } catch (error) { + errors++; + const message = error instanceof Error ? error.message : String(error); + console.error(` ✗ Error on PR #${pr.number}: ${message}`); + } + } + + // ── 5. Summary ─────────────────────────────────────────────────────────── + console.log('\n=== Summary ==='); + console.log(` PRs processed: ${prs.length}`); + console.log(` Labels changed: ${changed}`); + console.log(` Labels already correct: ${skipped}`); + console.log(` Errors: ${errors}`); + + if (errors > 0) { + process.exitCode = 1; + } +}; diff --git a/.github/scripts/review-sync/tests/test-labels.js b/.github/scripts/review-sync/tests/test-labels.js new file mode 100644 index 000000000..e04181dc2 --- /dev/null +++ b/.github/scripts/review-sync/tests/test-labels.js @@ -0,0 +1,165 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// tests/test-labels.js +// +// Unit tests for helpers/labels.js (determineLabel, ensureLabel, syncLabel). +// Run with: node .github/scripts/review-sync/tests/test-labels.js + +const { runTestSuite, createMockGithub } = require('./test-utils'); +const { determineLabel, ensureLabel, syncLabel } = require('../helpers/labels'); +const { QUEUE_LABELS } = require('../helpers/constants'); + +const unitTests = [ + { + name: 'determineLabel: 0 approvals → queue:junior-committer', + test: () => { + const r = determineLabel({ maintainerApproval: 0, writeApproval: 0, softApproval: 0, anyApproval: 0 }); + return r.name === 'queue:junior-committer'; + }, + }, + { + name: 'determineLabel: 1 soft approval → queue:committers', + test: () => { + const r = determineLabel({ maintainerApproval: 0, writeApproval: 0, softApproval: 1, anyApproval: 1 }); + return r.name === 'queue:committers'; + }, + }, + { + name: 'determineLabel: 1 write approval → queue:maintainers', + test: () => { + const r = determineLabel({ maintainerApproval: 0, writeApproval: 1, softApproval: 0, anyApproval: 1 }); + return r.name === 'queue:maintainers'; + }, + }, + { + name: 'determineLabel: 2 write + 0 maintainer → queue:maintainers (NOT status: ready-to-merge)', + test: () => { + const r = determineLabel({ maintainerApproval: 0, writeApproval: 2, softApproval: 0, anyApproval: 2 }); + return r.name === 'queue:maintainers'; + }, + }, + { + name: 'determineLabel: 1 maintainer alone → queue:maintainers (NOT status: ready-to-merge, needs 2 reviews)', + test: () => { + // Sophie's edge case: maintainer approves first, only 1 total review + const r = determineLabel({ maintainerApproval: 1, writeApproval: 0, softApproval: 0, anyApproval: 1 }); + return r.name === 'queue:maintainers'; + }, + }, + { + name: 'determineLabel: 1 maintainer + 1 write → status: ready-to-merge (2 reviews satisfied)', + test: () => { + const r = determineLabel({ maintainerApproval: 1, writeApproval: 1, softApproval: 0, anyApproval: 2 }); + return r.name === 'status: ready-to-merge'; + }, + }, + { + name: 'determineLabel: 1 maintainer + 1 soft → queue:maintainers (soft approvals do not count as core review)', + test: () => { + const r = determineLabel({ maintainerApproval: 1, writeApproval: 0, softApproval: 1, anyApproval: 2 }); + return r.name === 'queue:maintainers'; + }, + }, + { + name: 'determineLabel: 3 soft, 0 write → queue:committers', + test: () => { + const r = determineLabel({ maintainerApproval: 0, writeApproval: 0, softApproval: 3, anyApproval: 3 }); + return r.name === 'queue:committers'; + }, + }, + { + name: 'ensureLabel: creates label when missing', + test: async () => { + const mock = createMockGithub({ existingLabels: {} }); + await ensureLabel(mock, 'o', 'r', QUEUE_LABELS.JUNIOR, false); + return mock.calls.labelsCreated.length === 1 && mock.calls.labelsCreated[0].name === 'queue:junior-committer'; + }, + }, + { + name: 'ensureLabel: skips when label exists', + test: async () => { + const mock = createMockGithub({ existingLabels: { 'queue:junior-committer': true } }); + await ensureLabel(mock, 'o', 'r', QUEUE_LABELS.JUNIOR, false); + return mock.calls.labelsCreated.length === 0; + }, + }, + { + name: 'ensureLabel: dry run does not create', + test: async () => { + const mock = createMockGithub({ existingLabels: {} }); + await ensureLabel(mock, 'o', 'r', QUEUE_LABELS.JUNIOR, true); + return mock.calls.labelsCreated.length === 0; + }, + }, + { + name: 'syncLabel: no approvals, no labels → adds junior-committer', + test: async () => { + const mock = createMockGithub({ roles: {}, reviews: [] }); + const changed = await syncLabel(mock, 'o', 'r', { number: 1, labels: [] }, false); + return changed === true && mock.calls.labelsAdded[0] === 'queue:junior-committer'; + }, + }, + { + name: 'syncLabel: already correct, no stale → returns false', + test: async () => { + const mock = createMockGithub({ roles: {}, reviews: [] }); + const changed = await syncLabel(mock, 'o', 'r', { number: 1, labels: [{ name: 'queue:junior-committer' }] }, false); + return changed === false && mock.calls.labelsAdded.length === 0; + }, + }, + { + name: 'syncLabel: stale label → adds correct, removes stale', + test: async () => { + const mock = createMockGithub({ + roles: { + sophie: { role_name: 'maintain', permission: 'write' }, + bob: { role_name: 'write', permission: 'write' }, + }, + reviews: [ + { user: { login: 'sophie' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'bob' }, state: 'APPROVED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const changed = await syncLabel(mock, 'o', 'r', { number: 1, labels: [{ name: 'queue:junior-committer' }] }, false); + return changed === true && mock.calls.labelsAdded.includes('status: ready-to-merge') && mock.calls.labelsRemoved.includes('queue:junior-committer'); + }, + }, + { + name: 'syncLabel: dry run logs but does not modify', + test: async () => { + const mock = createMockGithub({ roles: {}, reviews: [] }); + const changed = await syncLabel(mock, 'o', 'r', { number: 1, labels: [{ name: 'queue:committers' }] }, true); + return changed === true && mock.calls.labelsAdded.length === 0; + }, + }, + { + name: 'syncLabel: correct + stale present → cleans up stale', + test: async () => { + const mock = createMockGithub({ roles: {}, reviews: [] }); + const pr = { number: 1, labels: [{ name: 'queue:junior-committer' }, { name: 'queue:committers' }] }; + const changed = await syncLabel(mock, 'o', 'r', pr, false); + return changed === true && mock.calls.labelsRemoved.includes('queue:committers'); + }, + }, +]; + +async function runUnitTests() { + console.log('🔬 UNIT TESTS (labels)'); + console.log('='.repeat(70)); + let passed = 0; + let failed = 0; + for (const t of unitTests) { + try { + const result = await Promise.resolve(t.test()); + if (result) { console.log(`✅ ${t.name}`); passed++; } + else { console.log(`❌ ${t.name}`); failed++; } + } catch (error) { console.log(`❌ ${t.name} - Error: ${error.message}`); failed++; } + } + console.log('\n' + '-'.repeat(70)); + console.log(`Unit Tests: ${passed} passed, ${failed} failed`); + return { total: unitTests.length, passed, failed }; +} + +runTestSuite('LABELS TEST SUITE', [], async () => true, [ + { label: 'Unit Tests', run: runUnitTests }, +]); diff --git a/.github/scripts/review-sync/tests/test-permissions.js b/.github/scripts/review-sync/tests/test-permissions.js new file mode 100644 index 000000000..b785a036e --- /dev/null +++ b/.github/scripts/review-sync/tests/test-permissions.js @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// tests/test-permissions.js +// +// Unit tests for helpers/permissions.js (getPermissionLevel, countApprovals). +// Run with: node .github/scripts/review-sync/tests/test-permissions.js + +const { runTestSuite, createMockGithub } = require('./test-utils'); +const { getPermissionLevel, countApprovals } = require('../helpers/permissions'); + +// ============================================================================= +// UNIT TESTS +// ============================================================================= + +const unitTests = [ + // --------------------------------------------------------------------------- + // getPermissionLevel + // --------------------------------------------------------------------------- + { + name: 'getPermissionLevel: uses role_name over permission (maintain case)', + test: async () => { + const mock = createMockGithub({ + roles: { sophie: { role_name: 'maintain', permission: 'write' } }, + }); + const result = await getPermissionLevel(mock, 'owner', 'repo', 'sophie'); + return result === 'maintain'; // NOT 'write' + }, + }, + { + name: 'getPermissionLevel: uses role_name over permission (admin case)', + test: async () => { + const mock = createMockGithub({ + roles: { admin: { role_name: 'admin', permission: 'admin' } }, + }); + const result = await getPermissionLevel(mock, 'owner', 'repo', 'admin'); + return result === 'admin'; + }, + }, + { + name: 'getPermissionLevel: falls back to permission if role_name missing', + test: async () => { + const mock = createMockGithub({ + roles: { bob: { permission: 'write' } }, + }); + const result = await getPermissionLevel(mock, 'owner', 'repo', 'bob'); + return result === 'write'; + }, + }, + { + name: 'getPermissionLevel: external contributor (404) returns none', + test: async () => { + const mock = createMockGithub({ roles: {} }); + const result = await getPermissionLevel(mock, 'owner', 'repo', 'unknown-user'); + return result === 'none'; + }, + }, + { + name: 'getPermissionLevel: triage role returned correctly', + test: async () => { + const mock = createMockGithub({ + roles: { triager: { role_name: 'triage', permission: 'read' } }, + }); + const result = await getPermissionLevel(mock, 'owner', 'repo', 'triager'); + return result === 'triage'; + }, + }, + { + name: 'getPermissionLevel: read role returned correctly', + test: async () => { + const mock = createMockGithub({ + roles: { reader: { role_name: 'read', permission: 'read' } }, + }); + const result = await getPermissionLevel(mock, 'owner', 'repo', 'reader'); + return result === 'read'; + }, + }, + + // --------------------------------------------------------------------------- + // countApprovals + // --------------------------------------------------------------------------- + { + name: 'countApprovals: maintain counted as maintainerApproval, write as writeApproval', + test: async () => { + const mock = createMockGithub({ + roles: { + sophie: { role_name: 'maintain', permission: 'write' }, + bob: { role_name: 'write', permission: 'write' }, + }, + reviews: [ + { user: { login: 'sophie' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'bob' }, state: 'APPROVED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return ( + result.maintainerApproval === 1 && + result.writeApproval === 1 && + result.softApproval === 0 && + result.anyApproval === 2 + ); + }, + }, + { + name: 'countApprovals: admin counted as maintainerApproval', + test: async () => { + const mock = createMockGithub({ + roles: { + admin: { role_name: 'admin', permission: 'admin' }, + }, + reviews: [ + { user: { login: 'admin' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + ], + }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return result.maintainerApproval === 1 && result.anyApproval === 1; + }, + }, + { + name: 'countApprovals: external contributor counted as softApproval', + test: async () => { + const mock = createMockGithub({ + roles: {}, + reviews: [ + { user: { login: 'external' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + ], + }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return ( + result.maintainerApproval === 0 && + result.writeApproval === 0 && + result.softApproval === 1 && + result.anyApproval === 1 + ); + }, + }, + { + name: 'countApprovals: CHANGES_REQUESTED not counted in any counter', + test: async () => { + const mock = createMockGithub({ + roles: { + bob: { role_name: 'write', permission: 'write' }, + }, + reviews: [ + { user: { login: 'bob' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-01T00:00:00Z' }, + ], + }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return result.anyApproval === 0; + }, + }, + { + name: 'countApprovals: no reviews returns all zeros', + test: async () => { + const mock = createMockGithub({ roles: {}, reviews: [] }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return ( + result.maintainerApproval === 0 && + result.writeApproval === 0 && + result.softApproval === 0 && + result.anyApproval === 0 + ); + }, + }, + { + name: 'countApprovals: mixed roles — 1 admin + 1 write + 1 external', + test: async () => { + const mock = createMockGithub({ + roles: { + admin: { role_name: 'admin', permission: 'admin' }, + committer: { role_name: 'write', permission: 'write' }, + }, + reviews: [ + { user: { login: 'admin' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'committer' }, state: 'APPROVED', submitted_at: '2026-01-02T00:00:00Z' }, + { user: { login: 'random' }, state: 'APPROVED', submitted_at: '2026-01-03T00:00:00Z' }, + ], + }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return ( + result.maintainerApproval === 1 && + result.writeApproval === 1 && + result.softApproval === 1 && + result.anyApproval === 3 + ); + }, + }, + { + name: 'countApprovals: DISMISSED approval is not counted', + test: async () => { + const mock = createMockGithub({ + roles: { + sophie: { role_name: 'maintain', permission: 'write' }, + }, + reviews: [ + { user: { login: 'sophie' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'sophie' }, state: 'DISMISSED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const result = await countApprovals(mock, 'owner', 'repo', 1); + return result.maintainerApproval === 0 && result.anyApproval === 0; + }, + }, +]; + +// ============================================================================= +// TEST RUNNER +// ============================================================================= + +async function runUnitTests() { + console.log('🔬 UNIT TESTS (permissions)'); + console.log('='.repeat(70)); + let passed = 0; + let failed = 0; + for (const test of unitTests) { + try { + const result = await Promise.resolve(test.test()); + if (result) { + console.log(`✅ ${test.name}`); + passed++; + } else { + console.log(`❌ ${test.name}`); + failed++; + } + } catch (error) { + console.log(`❌ ${test.name} - Error: ${error.message}`); + failed++; + } + } + console.log('\n' + '-'.repeat(70)); + console.log(`Unit Tests: ${passed} passed, ${failed} failed`); + return { total: unitTests.length, passed, failed }; +} + +runTestSuite('PERMISSIONS TEST SUITE', [], async () => true, [ + { label: 'Unit Tests', run: runUnitTests }, +]); diff --git a/.github/scripts/review-sync/tests/test-reviews.js b/.github/scripts/review-sync/tests/test-reviews.js new file mode 100644 index 000000000..2dab3c725 --- /dev/null +++ b/.github/scripts/review-sync/tests/test-reviews.js @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// tests/test-reviews.js +// +// Unit tests for helpers/reviews.js (getLatestReviewStates). +// Run with: node .github/scripts/review-sync/tests/test-reviews.js + +const { runTestSuite, createMockGithub } = require('./test-utils'); +const { getLatestReviewStates } = require('../helpers/reviews'); + +const unitTests = [ + { + name: 'getLatestReviewStates: single APPROVED → returns approved', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'alice' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.size === 1 && states.get('alice') === 'APPROVED'; + }, + }, + { + name: 'getLatestReviewStates: DISMISSED deletes prior APPROVED', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'alice' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'alice' }, state: 'DISMISSED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.size === 0; + }, + }, + { + name: 'getLatestReviewStates: COMMENTED is ignored', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'bob' }, state: 'COMMENTED', submitted_at: '2026-01-01T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.size === 0; + }, + }, + { + name: 'getLatestReviewStates: later APPROVED overwrites CHANGES_REQUESTED', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'alice' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'alice' }, state: 'APPROVED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.get('alice') === 'APPROVED'; + }, + }, + { + name: 'getLatestReviewStates: later CHANGES_REQUESTED overwrites APPROVED', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'alice' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'alice' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.get('alice') === 'CHANGES_REQUESTED'; + }, + }, + { + name: 'getLatestReviewStates: multiple reviewers tracked independently', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'alice' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'bob' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.size === 2 && states.get('alice') === 'APPROVED' && states.get('bob') === 'CHANGES_REQUESTED'; + }, + }, + { + name: 'getLatestReviewStates: no reviews → empty map', + test: async () => { + const mock = createMockGithub({ reviews: [] }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.size === 0; + }, + }, + { + name: 'getLatestReviewStates: null login or state gracefully skipped', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: null, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' }, + { user: { login: 'bob' }, state: null, submitted_at: '2026-01-02T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + return states.size === 0; + }, + }, + { + name: 'getLatestReviewStates: out-of-order timestamps sorted correctly', + test: async () => { + const mock = createMockGithub({ + reviews: [ + { user: { login: 'alice' }, state: 'APPROVED', submitted_at: '2026-01-05T00:00:00Z' }, + { user: { login: 'alice' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-01T00:00:00Z' }, + ], + }); + const states = await getLatestReviewStates(mock, 'o', 'r', 1); + // After sorting, CHANGES_REQUESTED is first, APPROVED is second → final state is APPROVED + return states.get('alice') === 'APPROVED'; + }, + }, +]; + +async function runUnitTests() { + console.log('🔬 UNIT TESTS (reviews)'); + console.log('='.repeat(70)); + let passed = 0; + let failed = 0; + for (const t of unitTests) { + try { + const result = await Promise.resolve(t.test()); + if (result) { console.log(`✅ ${t.name}`); passed++; } + else { console.log(`❌ ${t.name}`); failed++; } + } catch (error) { console.log(`❌ ${t.name} - Error: ${error.message}`); failed++; } + } + console.log('\n' + '-'.repeat(70)); + console.log(`Unit Tests: ${passed} passed, ${failed} failed`); + return { total: unitTests.length, passed, failed }; +} + +runTestSuite('REVIEWS TEST SUITE', [], async () => true, [ + { label: 'Unit Tests', run: runUnitTests }, +]); diff --git a/.github/scripts/review-sync/tests/test-utils.js b/.github/scripts/review-sync/tests/test-utils.js new file mode 100644 index 000000000..7f4aa7cc9 --- /dev/null +++ b/.github/scripts/review-sync/tests/test-utils.js @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// tests/test-utils.js +// +// Shared test utilities for review-sync test suites. +// Adapted from hiero-sdk-cpp/.github/scripts/tests/test-utils.js +// +// Provides: +// - runTestSuite() — CLI-aware test runner with summary +// - printSummaryAndExit() — formatted results + exit code +// - createMockGithub() — mock GitHub API factory with call tracking + +// ============================================================================= +// SUMMARY & RUNNER +// ============================================================================= + +/** + * Prints a summary table and exits with the appropriate code. + * + * @param {{ label: string, total: number, passed: number, failed: number }[]} sections + */ +function printSummaryAndExit(sections) { + console.log('\n' + '='.repeat(70)); + console.log('📈 SUMMARY'); + console.log('='.repeat(70)); + + let anyFailed = false; + for (const { label, total, passed, failed } of sections) { + if (failed > 0) anyFailed = true; + console.log( + ` ${label}: ${total} total, ${passed} passed` + + `${failed > 0 ? `, ${failed} failed ❌` : ' ✅'}` + ); + } + + console.log('='.repeat(70)); + process.exit(anyFailed ? 1 : 0); +} + +/** + * Parses the optional test-index CLI argument and either runs a single + * test or all tests, then prints a summary and exits. + * + * @param {string} suiteName - Display name (e.g. "PERMISSIONS TEST SUITE") + * @param {object[]} scenarios - Array of scenario objects (unused for unit-only suites, pass []) + * @param {function} runScenario - Async function that runs one scenario (pass async () => true for unit-only) + * @param {{ label: string, run: () => Promise<{ total: number, passed: number, failed: number }> }[]} extraSections + */ +async function runTestSuite(suiteName, scenarios, runScenario, extraSections = []) { + console.log(`🧪 ${suiteName}`); + console.log('='.repeat(suiteName.length + 3) + '\n'); + + const summaries = []; + + for (const section of extraSections) { + const result = await section.run(); + summaries.push({ label: section.label, ...result }); + } + + // Only run integration scenarios if there are any + if (scenarios.length > 0) { + console.log('\n\n🔗 INTEGRATION TESTS'); + console.log('='.repeat(70)); + + let passed = 0; + let failed = 0; + for (let i = 0; i < scenarios.length; i++) { + const ok = await runScenario(scenarios[i], i); + if (ok) passed++; + else failed++; + } + summaries.push({ label: 'Integration Tests', total: scenarios.length, passed, failed }); + } + + printSummaryAndExit(summaries); +} + +// ============================================================================= +// MOCK GITHUB FACTORY +// ============================================================================= + +/** + * Creates a mock GitHub API object for review-sync tests. + * Tracks labels, permission checks, and review fetches via the returned calls object. + * + * @param {object} options + * @param {Record} options.roles + * Map of username → permission data returned by getCollaboratorPermissionLevel + * @param {Array} options.reviews + * Array of review objects returned by pulls.listReviews + * @param {Record} options.existingLabels + * Map of label name → true (label exists in repo) + * @returns {{ calls: object, rest: object, paginate: function }} + */ +function createMockGithub(options = {}) { + const { + roles = {}, + reviews = [], + existingLabels = {}, + } = options; + + const calls = { + labelsAdded: [], + labelsRemoved: [], + labelsCreated: [], + labelsChecked: [], + permissionsChecked: [], + }; + + const mock = { + calls, + rest: { + repos: { + getCollaboratorPermissionLevel: async ({ username }) => { + calls.permissionsChecked.push(username); + const role = roles[username]; + if (!role) { + throw Object.assign(new Error('Not found'), { status: 404 }); + } + return { data: role }; + }, + }, + pulls: { + listReviews: async () => ({ data: reviews }), + }, + issues: { + getLabel: async ({ name }) => { + calls.labelsChecked.push(name); + if (!existingLabels[name]) { + throw Object.assign(new Error('Not found'), { status: 404 }); + } + return { data: { name } }; + }, + createLabel: async ({ name, color, description }) => { + calls.labelsCreated.push({ name, color, description }); + return {}; + }, + addLabels: async ({ labels }) => { + calls.labelsAdded.push(...labels); + return {}; + }, + removeLabel: async ({ name }) => { + calls.labelsRemoved.push(name); + return {}; + }, + }, + rateLimit: { + get: async () => ({ + data: { resources: { core: { remaining: 5000 } } }, + }), + }, + }, + paginate: async (fn, opts) => { + const result = await fn(opts); + return result.data || result || []; + }, + }; + + return mock; +} + +module.exports = { runTestSuite, printSummaryAndExit, createMockGithub }; diff --git a/.github/workflows/review-sync.yml b/.github/workflows/review-sync.yml new file mode 100644 index 000000000..9799fdbf4 --- /dev/null +++ b/.github/workflows/review-sync.yml @@ -0,0 +1,58 @@ +# Automated PR review queue label sync. +# Runs on a 30-minute schedule to classify every open non-draft PR into one of +# four review stages based on the number and permission level of approvals: +# queue:junior-committer → queue:committers → queue:maintainers → ready-to-merge +# +# Phase 1 of 4 — label sync only. No assignments, no comments, no routing. +# +# Old step:/reviewer: labels are intentionally left untouched and will coexist +# with the new queue: labels until a future cleanup PR removes them. +# Bot-authored PRs (e.g. Dependabot) are included and will receive labels. + +name: Review Queue Label Sync + +on: + schedule: + - cron: '*/30 * * * *' + workflow_dispatch: + inputs: + dry_run: + description: 'If true, log label changes without applying them. Default true for manual runs.' + required: false + default: 'true' + type: string + +permissions: + pull-requests: read + issues: write + contents: read + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: true + +jobs: + sync: + runs-on: hl-sdk-py-lin-md + env: + DRY_RUN: ${{ inputs.dry_run || 'false' }} + + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0 + with: + egress-policy: audit + + - name: Checkout scripts + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + sparse-checkout: | + .github/scripts + sparse-checkout-cone-mode: false + + - name: Run review sync + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const fn = require('./.github/scripts/review-sync/index.js'); + await fn({ github, context, core });