diff --git a/.github/policies/resourceManagement.yml b/.github/policies/resourceManagement.yml index a785713bc2..ce5da93a1a 100644 --- a/.github/policies/resourceManagement.yml +++ b/.github/policies/resourceManagement.yml @@ -73,22 +73,6 @@ configuration: - addLabel: label: Needs-Team-Response description: - # ─── When PR author pushes commits, swap Needs-Author-Feedback → Needs-Triage ─── - - if: - - payloadType: Pull_Request - - isAction: - action: Synchronize - - isActivitySender: - issueAuthor: True - - hasLabel: - label: Needs-Author-Feedback - - isOpen - then: - - addLabel: - label: Needs-Triage - - removeLabel: - label: Needs-Author-Feedback - description: # ─── Remove "Status-No recent activity" on any issue update (not close) ─── - if: - payloadType: Issues diff --git a/.github/scripts/issue-triage/README.md b/.github/scripts/issue-triage/README.md index 6433f0a2bc..d5488fb248 100644 --- a/.github/scripts/issue-triage/README.md +++ b/.github/scripts/issue-triage/README.md @@ -49,7 +49,7 @@ that retain `Needs-Author-Feedback` for seven days without activity. - An author comment removes `Needs-Author-Feedback` and returns the item to team triage. -- A PR push removes `Needs-Author-Feedback`. +- A PR push reruns PR intake, which recalculates `Needs-Author-Feedback`. - Manual label removal immediately makes the item ineligible for scheduled closure. diff --git a/.github/scripts/pr-intake/README.md b/.github/scripts/pr-intake/README.md new file mode 100644 index 0000000000..539210124b --- /dev/null +++ b/.github/scripts/pr-intake/README.md @@ -0,0 +1,47 @@ +# Pull request intake + +The workflow in `.github/workflows/pr-intake.yml` runs deterministic pull +request intake checks on every pull request. It does not call any AI model and +does not execute any code from the pull request head; the Node script reads all +pull request data through the GitHub API. + +## Flow + +1. Read the current PR through the GitHub API. Mergeability is re-fetched a few + times when GitHub still reports it as unknown so a conflicting PR is never + treated as ready by default. +2. Deterministically validate closing issue references, merge conflicts, draft + state, and whether visual evidence is present. +3. Require visual evidence only when the changed paths touch product UI files. +4. Keep a single canonical comment in sync and manage only the `Needs-Review` + and `Needs-Author-Feedback` labels. + +## Comment behavior + +- If there is anything for the author to act on or consider, a single canonical + comment is posted or updated. +- If the PR is clean and a previous intake comment exists, it is replaced with a + short all-clear note. +- If the PR is clean and no intake comment exists, nothing is posted. + +Missing issue references are advisory. Explicitly invalid references, merge +conflicts, unknown mergeability, and missing required visual evidence block +readiness. Draft PRs do not receive `Needs-Author-Feedback` solely because they +are drafts. + +The existing resource-management policy closes PRs that retain +`Needs-Author-Feedback` for seven inactive days. PR synchronization is handled +by this workflow rather than the legacy policy responder. + +## Robustness limits + +- Closing references parsed from the untrusted PR body are capped + (`MAX_CLOSING_REFERENCES`) and verified with bounded concurrency + (`CLOSING_VERIFY_CONCURRENCY`) so a crafted body cannot exhaust the API rate + limit. + +Run the focused tests with: + +```console +node --test .github\scripts\pr-intake\tests\pr-intake.test.mjs +``` diff --git a/.github/scripts/pr-intake/pr-intake.mjs b/.github/scripts/pr-intake/pr-intake.mjs new file mode 100644 index 0000000000..d30c57da92 --- /dev/null +++ b/.github/scripts/pr-intake/pr-intake.mjs @@ -0,0 +1,955 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +export const CANONICAL_MARKER = ''; +export const ACTIONS_BOT_LOGIN = 'github-actions[bot]'; +export const ACTIONS_BOT_ID = 41898282; +export const READY_FOR_REVIEW_LABEL = 'Needs-Review'; +export const NEEDS_AUTHOR_FEEDBACK_LABEL = 'Needs-Author-Feedback'; +export const FEEDBACK_SINCE_MARKER = 'powertoys-pr-intake:feedback-since'; +export const STALE_FEEDBACK_DAYS = 7; + +const PAGE_SIZE = 100; +const MAX_COMMENT_PAGES = 10; +const MAX_FILE_PAGES = 10; +// Closing references come from the untrusted PR body. Cap how many we accept and +// verify them with bounded concurrency so a crafted body cannot fan out into +// thousands of concurrent API calls and exhaust the token's rate limit. +const MAX_CLOSING_REFERENCES = 20; +const CLOSING_VERIFY_CONCURRENCY = 5; +// GitHub computes mergeability asynchronously and returns `mergeable: null` +// meanwhile. Re-fetch a few times before treating the state as known. +const MERGEABILITY_MAX_ATTEMPTS = 5; +const MERGEABILITY_RETRY_DELAY_MS = 2000; + +const VISUAL_FILE_EXTENSIONS = new Set([ + '.axaml', + '.css', + '.gif', + '.html', + '.ico', + '.jpeg', + '.jpg', + '.png', + '.svg', + '.webp', + '.xaml', +]); + +const VISUAL_PRODUCT_PREFIXES = [ + 'src/modules/', + 'src/runner/', + 'src/settings-ui/', +]; + +export class ApiError extends Error { + constructor(message, status, details = '') { + super(message); + this.name = 'ApiError'; + this.status = status; + this.details = details; + } +} + +function boundedString(value, maxLength, fallback = '') { + if (typeof value !== 'string') { + return fallback; + } + const normalized = value.replace(/\u0000/g, '').trim(); + return normalized.slice(0, maxLength); +} + +function markdownPlainText(value, maxLength, fallback = '') { + return boundedString(value, maxLength, fallback) + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('@', '@\u200b') + .replace(/[\r\n]+/g, ' ') + .replace(/([\\`*_[\]])/g, '\\$1'); +} + +function formatInlineCode(value) { + return `\`${boundedString(String(value).replaceAll('`', '').replace(/\r?\n+/g, ' '), 500)}\``; +} + +export function deriveVisualAssessment(requiresVisualEvidence) { + return requiresVisualEvidence + ? { + visualEvidenceRequirement: 'REQUIRED', + visualEvidenceReason: + 'The pull request changes product UI files, so reviewers need to see the visible result.', + } + : { + visualEvidenceRequirement: 'NOT_NEEDED', + visualEvidenceReason: + 'The changed files do not indicate a visible UI change.', + }; +} + +function sleep(milliseconds) { + return new Promise((resolve) => { + setTimeout(resolve, Math.max(0, milliseconds)); + }); +} + +async function mapWithConcurrency(items, limit, mapper) { + const list = Array.isArray(items) ? items : []; + const results = new Array(list.length); + const boundedLimit = Math.max(1, Math.min(limit, list.length || 1)); + let cursor = 0; + async function worker() { + while (cursor < list.length) { + const index = cursor; + cursor += 1; + results[index] = await mapper(list[index], index); + } + } + await Promise.all( + Array.from({ length: boundedLimit }, () => worker()), + ); + return results; +} + +function uniqueSorted(values) { + return [...new Set(values.filter(Boolean))] + .sort((left, right) => left.localeCompare(right)); +} + +export function normalizePath(value) { + if (typeof value !== 'string') { + return ''; + } + return value + .replace(/\\/g, '/') + .replace(/^\.\//, '') + .replace(/^\/+/, '') + .trim(); +} + +function isTestPath(changedPath) { + return /(^|\/)(test|tests|unittests|uitests)(\/|$)/i.test(changedPath) + || /\.(?:spec|test)\.[^.]+$/i.test(changedPath); +} + +function classifyPath(changedPath) { + const extension = path.extname(changedPath).toLowerCase(); + if ( + changedPath.endsWith('.md') + || changedPath.startsWith('doc/') + || changedPath.startsWith('docs/') + ) { + return 'docs'; + } + if (isTestPath(changedPath)) { + return 'tests'; + } + if ( + changedPath.startsWith('.github/') + || changedPath.startsWith('.pipelines/') + || changedPath.startsWith('tools/') + || changedPath.startsWith('installer/') + ) { + return 'infrastructure'; + } + if ( + VISUAL_FILE_EXTENSIONS.has(extension) + && VISUAL_PRODUCT_PREFIXES.some((prefix) => changedPath.startsWith(prefix)) + ) { + return 'product-ui'; + } + return 'product-code'; +} + +export function classifyChangedPaths(paths) { + if (!Array.isArray(paths)) { + throw new Error('Changed paths must be an array'); + } + const normalizedPaths = uniqueSorted( + paths.map((entry) => normalizePath(entry)).filter(Boolean), + ); + const pathCategories = normalizedPaths.map((changedPath) => ({ + changedPath, + category: classifyPath(changedPath), + })); + const categories = uniqueSorted(pathCategories.map((entry) => entry.category)); + const visualCandidatePaths = pathCategories + .filter((entry) => entry.category === 'product-ui') + .map((entry) => entry.changedPath); + + return { + changedPathCount: normalizedPaths.length, + normalizedPaths, + categories, + visualCandidatePaths, + requiresVisualEvidence: visualCandidatePaths.length > 0, + }; +} + +export function findClosingIssueReferences(body) { + const text = typeof body === 'string' ? body : ''; + const matches = []; + const seen = new Set(); + const regex = + /\b(closes?|closed|fixes?|fixed|resolves?|resolved)\s*:?\s+((?:[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)\s*)?#(\d+)\b/gi; + + for (const match of text.matchAll(regex)) { + const repositoryFullName = boundedString(match[2], 200).replace(/\s+/g, ''); + const issueNumber = Number(match[3]); + const seenKey = `${repositoryFullName.toLowerCase()}#${issueNumber}`; + if (!Number.isSafeInteger(issueNumber) || issueNumber <= 0 || seen.has(seenKey)) { + continue; + } + seen.add(seenKey); + matches.push({ + keyword: match[1].toLowerCase(), + issueNumber, + repositoryFullName: repositoryFullName || null, + }); + if (matches.length >= MAX_CLOSING_REFERENCES) { + break; + } + } + + return matches; +} + +function formatClosingReference(reference) { + return reference.repositoryFullName + ? `${reference.repositoryFullName}#${reference.issueNumber}` + : `#${reference.issueNumber}`; +} + +function invalidClosingReasonLabel(reason) { + switch (reason) { + case 'different-repository': + return 'different repo'; + case 'pull-request': + return 'pull request'; + case 'not-found': + return 'not found'; + default: + return 'invalid'; + } +} + +function formatInvalidClosingReference(reference) { + return `${formatInlineCode(formatClosingReference(reference))} (${invalidClosingReasonLabel(reference.reason)})`; +} + +export async function verifyClosingIssueReferences({ + api, + repositoryFullName, + references, +}) { + const normalizedRepositoryFullName = boundedString(repositoryFullName, 200).toLowerCase(); + if (!normalizedRepositoryFullName) { + throw new Error('Repository full name is required for closing issue verification'); + } + + const boundedReferences = (Array.isArray(references) ? references : []) + .slice(0, MAX_CLOSING_REFERENCES); + const results = await mapWithConcurrency( + boundedReferences, + CLOSING_VERIFY_CONCURRENCY, + async (reference) => { + if (reference.repositoryFullName + && reference.repositoryFullName.toLowerCase() !== normalizedRepositoryFullName) { + return { + ...reference, + reason: 'different-repository', + }; + } + + try { + const issue = await api.getIssue(reference.issueNumber); + if (issue?.pull_request) { + return { + ...reference, + reason: 'pull-request', + }; + } + return { + ...reference, + title: boundedString(issue?.title, 300, `Issue ${reference.issueNumber}`), + }; + } catch (error) { + if (error instanceof ApiError && error.status === 404) { + return { + ...reference, + reason: 'not-found', + }; + } + throw error; + } + }, + ); + + return { + validReferences: results.filter((reference) => !reference.reason), + invalidReferences: results.filter((reference) => Boolean(reference.reason)), + }; +} + +function isVisualMediaUrl(value) { + const url = typeof value === 'string' ? value.trim() : ''; + if (!url) { + return false; + } + if (/^https:\/\/github\.com\/user-attachments\/assets\/[^\s<>)"]+/i.test(url)) { + return true; + } + return /\.(?:png|jpe?g|gif|webp|bmp|svg|mp4|mov|webm|m4v)(?:[?#].*)?$/i.test(url); +} + +export function findVisualEvidence(body) { + const text = typeof body === 'string' ? body : ''; + const evidenceTypes = []; + + const patterns = [ + { + type: 'GitHub user-attachment URL', + regex: /https:\/\/github\.com\/user-attachments\/(?:assets\/[^\s<>)"]+|files\/[^\s<>)"]+\.(?:png|jpe?g|gif|webp|bmp|svg|mp4|mov|webm|m4v))(?:[?#][^\s<>)"]*)?/gi, + }, + { + type: 'Recognized video link', + regex: /https?:\/\/(?:www\.)?(?:youtu\.be\/|youtube\.com\/watch\?[^\s<>"']*v=|vimeo\.com\/|loom\.com\/share\/|streamable\.com\/)[^\s<>)"]+/gi, + }, + ]; + + for (const { type, regex } of patterns) { + if (regex.test(text)) { + evidenceTypes.push(type); + } + } + + for (const match of text.matchAll( + /!\[[^\]]*]\(\s*(?:<([^>]+)>|([^)\s]+))(?:\s+"[^"]*")?\s*\)/gi, + )) { + if (isVisualMediaUrl(match[1] || match[2])) { + evidenceTypes.push('Markdown image'); + break; + } + } + + for (const match of text.matchAll( + /<(img|video|source)\b[^>]*\bsrc\s*=\s*['"]([^'"]+)['"][^>]*>/gi, + )) { + if (!isVisualMediaUrl(match[2])) { + continue; + } + evidenceTypes.push(match[1].toLowerCase() === 'img' + ? 'HTML image tag' + : 'HTML video tag'); + } + + return { + found: evidenceTypes.length > 0, + types: uniqueSorted(evidenceTypes), + }; +} + +export function isMergeabilityKnown(pullRequest) { + if (pullRequest?.mergeable === true || pullRequest?.mergeable === false) { + return true; + } + const mergeableState = boundedString( + pullRequest?.mergeable_state ?? pullRequest?.mergeStateStatus, + 40, + ).toLowerCase(); + return mergeableState !== '' && mergeableState !== 'unknown'; +} + +export function hasMergeConflict(pullRequest) { + const mergeableState = boundedString( + pullRequest?.mergeable_state ?? pullRequest?.mergeStateStatus, + 40, + ).toLowerCase(); + return pullRequest?.mergeable === false + || mergeableState === 'dirty' + || mergeableState === 'conflicting'; +} + +// GitHub returns `mergeable: null` / `mergeable_state: unknown` while it is still +// computing mergeability. Re-fetch until the state is known so a conflicting PR +// is never treated as ready by default. +export async function getPullRequestWithMergeability( + api, + pullNumber, + { + maxAttempts = MERGEABILITY_MAX_ATTEMPTS, + delayMs = MERGEABILITY_RETRY_DELAY_MS, + sleepImpl = sleep, + } = {}, +) { + let pullRequest = await api.getPullRequest(pullNumber); + let attempt = 1; + while (!isMergeabilityKnown(pullRequest) && attempt < maxAttempts) { + await sleepImpl(delayMs); + pullRequest = await api.getPullRequest(pullNumber); + attempt += 1; + } + return { + pullRequest, + mergeabilityKnown: isMergeabilityKnown(pullRequest), + }; +} + +function buildContributingUrl(repositoryHtmlUrl, baseRef) { + const repoUrl = boundedString(repositoryHtmlUrl, 500); + const branch = boundedString(baseRef, 200, 'main'); + if (!repoUrl) { + throw new Error('Repository HTML URL is required to build CONTRIBUTING.md links'); + } + return `${repoUrl}/blob/${branch}/CONTRIBUTING.md`; +} + +export function buildIntakeReport({ + changedPaths, + body, + repositoryHtmlUrl, + baseRef, + authorLogin, + isDraft = false, + mergeConflict = false, + mergeabilityKnown = true, + verifiedClosingIssues, + invalidClosingIssues, +}) { + const ownership = classifyChangedPaths(changedPaths); + const visualAssessment = deriveVisualAssessment(ownership.requiresVisualEvidence); + const closingIssues = Array.isArray(verifiedClosingIssues) + ? verifiedClosingIssues + : findClosingIssueReferences(body); + const invalidClosingReferences = Array.isArray(invalidClosingIssues) + ? invalidClosingIssues + : []; + const visualEvidence = findVisualEvidence(body); + const authorActions = []; + const recommendations = []; + + if (mergeConflict) { + authorActions.push( + 'Resolve the merge conflicts with the target branch.', + ); + } + if (!closingIssues.length && !invalidClosingReferences.length) { + recommendations.push( + 'Link the issue this PR fixes using a closing keyword such as `Closes #123`.', + ); + } + if (invalidClosingReferences.length) { + authorActions.push( + `Replace the invalid closing reference${invalidClosingReferences.length === 1 ? '' : 's'} ${ + invalidClosingReferences.map((reference) => formatInvalidClosingReference(reference)).join(', ') + } with a valid issue, for example \`Closes #123\`.`, + ); + } + if ( + visualAssessment.visualEvidenceRequirement === 'REQUIRED' + && !visualEvidence.found + ) { + authorActions.push( + 'Add a screenshot, GIF, or video to the PR description so reviewers can validate the visible change.', + ); + } + const needsAuthorFeedback = authorActions.length > 0; + if (isDraft) { + authorActions.push('Mark the pull request as ready for review.'); + } + + return { + ...ownership, + closingIssues, + invalidClosingIssues: invalidClosingReferences, + visualEvidence, + visualAssessment, + authorActions, + recommendations, + authorLogin: boundedString(authorLogin, 100), + mergeConflict, + mergeabilityKnown, + needsAuthorFeedback, + readyForReview: authorActions.length === 0 && mergeabilityKnown, + contributingUrl: buildContributingUrl(repositoryHtmlUrl, baseRef), + }; +} + +export function parseFeedbackSince(body) { + const match = typeof body === 'string' + ? body.match(//) + : null; + if (!match) { + return null; + } + const timestamp = new Date(match[1]); + return Number.isNaN(timestamp.getTime()) ? null : timestamp.toISOString(); +} + +export function determineFeedbackSince({ + needsAuthorFeedback, + existingCommentBody, + action, + senderLogin, + authorLogin, + now = new Date(), +}) { + if (!needsAuthorFeedback) { + return null; + } + const currentTimestamp = now instanceof Date ? now : new Date(now); + if (Number.isNaN(currentTimestamp.getTime())) { + throw new Error('The feedback timestamp must be a valid date'); + } + const existing = parseFeedbackSince(existingCommentBody); + const normalizedSender = boundedString(senderLogin, 100).toLowerCase(); + const normalizedAuthor = boundedString(authorLogin, 100).toLowerCase(); + const isAuthorActivity = normalizedAuthor + && normalizedSender === normalizedAuthor + && action !== 'opened'; + return !existing || isAuthorActivity + ? currentTimestamp.toISOString() + : existing; +} + +export function renderIntakeComment(report, feedbackSince = null) { + const authorMention = /^[A-Za-z0-9-]+$/.test(report.authorLogin) + ? `@${report.authorLogin}, ` + : ''; + const feedbackMarker = report.needsAuthorFeedback && feedbackSince + ? `\n` + : ''; + const deadlineNotice = report.needsAuthorFeedback + ? `\nIf there is no author response within ${STALE_FEEDBACK_DAYS} days, this PR will be automatically closed.\n` + : ''; + const requirement = report.visualAssessment.visualEvidenceRequirement; + const visualEvidenceState = report.visualEvidence.found + ? 'Visual evidence was detected in the PR description.' + : requirement === 'REQUIRED' + ? 'Visual evidence is currently missing.' + : 'No visual evidence is expected.'; + const visualReason = markdownPlainText( + report.visualAssessment.visualEvidenceReason, + 500, + 'No explanation was provided.', + ); + const requirementLabel = requirement === 'REQUIRED' ? 'Required' : 'Not needed'; + const statusSection = report.readyForReview + ? `## ✅ Ready for review + +This PR passed the automated intake checks and is ready for maintainer review. +` + : `## Author action + +${authorMention}please update the following before review: + +${report.authorActions.map((entry) => `- ${entry}`).join('\n')} + +See the [contribution guide](${report.contributingUrl}) for the full checklist. +${deadlineNotice}`; + const recommendationSection = report.recommendations.length + ? `## Recommendation + +${report.recommendations.map((entry) => `> ${entry}`).join('\n')} + +` + : ''; + + return `${CANONICAL_MARKER} +## 🧭 PR intake${feedbackMarker} + +**Visual evidence:** ${requirementLabel} — ${visualReason} ${visualEvidenceState} + +${recommendationSection} +${statusSection} +_Automated PR intake; PowerToys maintainers make final decisions._ +`; +} + +export function renderAllClearComment() { + return `${CANONICAL_MARKER} +## ✅ PR intake + +All automated intake checks now pass. Thanks for the updates! + +_Automated PR intake; PowerToys maintainers make final decisions._ +`; +} + +export function selectCanonicalComment( + comments, + expectedLogin = ACTIONS_BOT_LOGIN, + expectedId = ACTIONS_BOT_ID, +) { + if (!Array.isArray(comments)) { + throw new Error('Comments must be an array'); + } + const trusted = comments.filter((comment) => + Number.isSafeInteger(comment?.id) + && comment.user?.login === expectedLogin + && comment.user?.type === 'Bot' + && (expectedId === null || comment.user?.id === expectedId) + && typeof comment.body === 'string' + && comment.body.includes(CANONICAL_MARKER)); + trusted.sort((left, right) => left.id - right.id); + return { + canonical: trusted[0] ?? null, + extras: trusted.slice(1), + }; +} + +async function listAllComments(api, issueNumber) { + const comments = []; + for (let page = 1; page <= MAX_COMMENT_PAGES; page += 1) { + const batch = await api.listIssueComments(issueNumber, page, PAGE_SIZE); + if (!Array.isArray(batch)) { + throw new Error('GitHub comments response must be an array'); + } + comments.push(...batch); + if (batch.length < PAGE_SIZE) { + return comments; + } + } + throw new Error(`Comment scan exceeded ${MAX_COMMENT_PAGES * PAGE_SIZE} items`); +} + +export async function upsertCanonicalComment({ + api, + issueNumber, + body, + comments = null, +}) { + const existingComments = Array.isArray(comments) + ? comments + : await listAllComments(api, issueNumber); + const { canonical, extras } = selectCanonicalComment(existingComments); + + let savedComment; + let operation; + if (canonical) { + savedComment = await api.updateIssueComment(canonical.id, body); + operation = 'updated'; + } else { + savedComment = await api.createIssueComment(issueNumber, body); + operation = 'created'; + } + + const deletedExtraComments = []; + for (const extra of extras) { + await api.deleteIssueComment(extra.id); + deletedExtraComments.push(extra.id); + } + + return { + comment: savedComment, + operation, + deletedExtraComments, + }; +} + +export function planManagedLabelChanges( + currentLabels, + desiredLabels, + managedLabels = [], +) { + const current = new Set(uniqueSorted( + (Array.isArray(currentLabels) ? currentLabels : []) + .map((entry) => typeof entry === 'string' ? entry : entry?.name) + .map((entry) => boundedString(entry, 100)) + .filter(Boolean), + )); + const desired = new Set(uniqueSorted( + (Array.isArray(desiredLabels) ? desiredLabels : []) + .map((entry) => boundedString(entry, 100)) + .filter(Boolean), + )); + const managed = new Set(uniqueSorted( + (Array.isArray(managedLabels) ? managedLabels : []) + .map((entry) => boundedString(entry, 100)) + .filter(Boolean), + )); + + return { + add: [...desired].filter((entry) => !current.has(entry)), + remove: [...current].filter((entry) => managed.has(entry) && !desired.has(entry)), + }; +} + +async function syncManagedLabels(api, issueNumber, labelPlan) { + for (const label of labelPlan.remove) { + await api.removeLabel(issueNumber, label); + } + if (labelPlan.add.length) { + await api.addLabels(issueNumber, labelPlan.add); + } +} + +async function listAllPullRequestFileDetails(api, pullNumber) { + const files = []; + for (let page = 1; page <= MAX_FILE_PAGES; page += 1) { + const batch = await api.listPullRequestFiles(pullNumber, page, PAGE_SIZE); + if (!Array.isArray(batch)) { + throw new Error('GitHub pull request files response must be an array'); + } + files.push(...batch); + if (batch.length < PAGE_SIZE) { + return files; + } + } + throw new Error(`Pull request file scan exceeded ${MAX_FILE_PAGES * PAGE_SIZE} items`); +} + +function changedPathsFromFileDetails(files) { + return (Array.isArray(files) ? files : []).flatMap((file) => [ + boundedString(file?.filename, 500), + boundedString(file?.previous_filename, 500), + ]).filter(Boolean); +} + +function parseIssueLabels(issue) { + return Array.isArray(issue?.labels) + ? issue.labels + .map((entry) => typeof entry === 'string' ? entry : entry?.name) + .filter((entry) => typeof entry === 'string' && entry.trim().length > 0) + : []; +} + +export async function runPullRequestIntake({ api, event }) { + if (!event?.repository) { + throw new Error('The GitHub event payload must contain repository data'); + } + + const pullNumber = Number( + event.pull_request?.number + ?? (event.issue?.pull_request ? event.issue.number : null), + ); + if (!Number.isSafeInteger(pullNumber) || pullNumber <= 0) { + throw new Error('The GitHub event payload must identify a pull request'); + } + const { pullRequest, mergeabilityKnown } = await getPullRequestWithMergeability( + api, + pullNumber, + ); + const issueNumber = pullNumber; + + const fileDetails = await listAllPullRequestFileDetails(api, issueNumber); + const changedPaths = changedPathsFromFileDetails(fileDetails); + const issue = await api.getIssue(issueNumber); + const closingReferenceVerification = await verifyClosingIssueReferences({ + api, + repositoryFullName: event.repository.full_name, + references: findClosingIssueReferences(pullRequest.body ?? ''), + }); + const report = buildIntakeReport({ + changedPaths, + body: pullRequest.body ?? '', + repositoryHtmlUrl: event.repository.html_url, + baseRef: pullRequest.base?.ref ?? 'main', + authorLogin: pullRequest.user?.login ?? '', + isDraft: pullRequest.draft === true, + mergeConflict: hasMergeConflict(pullRequest), + mergeabilityKnown, + verifiedClosingIssues: closingReferenceVerification.validReferences, + invalidClosingIssues: closingReferenceVerification.invalidReferences, + }); + const comments = await listAllComments(api, issueNumber); + const { canonical } = selectCanonicalComment(comments); + const feedbackSince = determineFeedbackSince({ + needsAuthorFeedback: report.needsAuthorFeedback, + existingCommentBody: canonical?.body ?? '', + action: event.action, + senderLogin: event.sender?.login, + authorLogin: pullRequest.user?.login, + }); + const desiredManagedLabels = [ + ...(report.readyForReview ? [READY_FOR_REVIEW_LABEL] : []), + ...(report.needsAuthorFeedback ? [NEEDS_AUTHOR_FEEDBACK_LABEL] : []), + ]; + const labelPlan = planManagedLabelChanges( + parseIssueLabels(issue), + desiredManagedLabels, + [ + READY_FOR_REVIEW_LABEL, + NEEDS_AUTHOR_FEEDBACK_LABEL, + ], + ); + + await syncManagedLabels(api, issueNumber, labelPlan); + + // Only surface a comment when there is something for the author to act on or + // consider. When a previously flagged PR becomes clean we replace the stale + // comment with a short all-clear note; when nothing was ever posted we stay + // silent to avoid noise on already-healthy PRs. + const hasRemarks = report.authorActions.length > 0 + || report.recommendations.length > 0; + let commentResult = { + operation: 'skipped', + comment: null, + deletedExtraComments: [], + }; + if (hasRemarks) { + commentResult = await upsertCanonicalComment({ + api, + issueNumber, + body: renderIntakeComment(report, feedbackSince), + comments, + }); + } else if (canonical) { + commentResult = await upsertCanonicalComment({ + api, + issueNumber, + body: renderAllClearComment(), + comments, + }); + } + + return { + issueNumber, + changedPathCount: changedPaths.length, + labelPlan, + commentResult: { + operation: commentResult.operation, + commentId: commentResult.comment?.id ?? null, + deletedExtraComments: commentResult.deletedExtraComments, + }, + requiresVisualEvidence: report.requiresVisualEvidence, + visualEvidenceRequirement: report.visualAssessment.visualEvidenceRequirement, + visualEvidenceFound: report.visualEvidence.found, + mergeConflict: report.mergeConflict, + mergeabilityKnown: report.mergeabilityKnown, + closingIssueCount: report.closingIssues.length, + invalidClosingIssueCount: report.invalidClosingIssues.length, + needsAuthorFeedback: report.needsAuthorFeedback, + feedbackSince, + readyForReview: report.readyForReview, + }; +} + +export class GitHubApi { + constructor({ + token, + owner, + repo, + apiBaseUrl = process.env.GITHUB_API_URL ?? 'https://api.github.com', + fetchImpl = fetch, + }) { + if (!token) { + throw new Error('GITHUB_TOKEN is required'); + } + if (!owner || !repo) { + throw new Error('Repository owner and name are required'); + } + this.token = token; + this.owner = owner; + this.repo = repo; + this.apiBaseUrl = apiBaseUrl.replace(/\/+$/, ''); + this.fetchImpl = fetchImpl; + } + + async request(method, route, body = undefined) { + const response = await this.fetchImpl( + `${this.apiBaseUrl}/repos/${this.owner}/${this.repo}${route}`, + { + method, + headers: { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${this.token}`, + 'Content-Type': 'application/json', + 'User-Agent': 'powertoys-pr-intake', + }, + body: body === undefined ? undefined : JSON.stringify(body), + }, + ); + + const raw = await response.text(); + const payload = raw ? JSON.parse(raw) : null; + if (!response.ok) { + throw new ApiError( + boundedString(payload?.message, 300, 'GitHub API request failed'), + response.status, + raw.slice(0, 500), + ); + } + return payload; + } + + listPullRequestFiles(pullNumber, page, perPage) { + return this.request( + 'GET', + `/pulls/${pullNumber}/files?page=${page}&per_page=${perPage}`, + ); + } + + listIssueComments(issueNumber, page, perPage) { + return this.request( + 'GET', + `/issues/${issueNumber}/comments?page=${page}&per_page=${perPage}`, + ); + } + + createIssueComment(issueNumber, body) { + return this.request('POST', `/issues/${issueNumber}/comments`, { body }); + } + + updateIssueComment(commentId, body) { + return this.request('PATCH', `/issues/comments/${commentId}`, { body }); + } + + deleteIssueComment(commentId) { + return this.request('DELETE', `/issues/comments/${commentId}`); + } + + getIssue(issueNumber) { + return this.request('GET', `/issues/${issueNumber}`); + } + + getPullRequest(pullNumber) { + return this.request('GET', `/pulls/${pullNumber}`); + } + + addLabels(issueNumber, labels) { + return this.request('POST', `/issues/${issueNumber}/labels`, { labels }); + } + + removeLabel(issueNumber, label) { + return this.request( + 'DELETE', + `/issues/${issueNumber}/labels/${encodeURIComponent(label)}`, + ); + } +} + +async function main() { + const eventPath = process.argv[2] || process.env.GITHUB_EVENT_PATH; + if (!eventPath) { + throw new Error('The GitHub event payload path is required'); + } + + const event = JSON.parse(await fs.readFile(eventPath, 'utf8')); + const repository = boundedString( + event?.repository?.full_name ?? process.env.GITHUB_REPOSITORY, + 200, + ); + const [owner, repo] = repository.split('/'); + if (!owner || !repo) { + throw new Error('Unable to determine the repository owner and name'); + } + + const api = new GitHubApi({ + token: process.env.GITHUB_TOKEN, + owner, + repo, + }); + + const result = await runPullRequestIntake({ api, event }); + console.log(JSON.stringify(result, null, 2)); +} + +const ENTRYPOINT = fileURLToPath(import.meta.url); +if (process.argv[1] && path.resolve(process.argv[1]) === ENTRYPOINT) { + main().catch((error) => { + console.error(error instanceof Error ? error.stack : String(error)); + process.exitCode = 1; + }); +} diff --git a/.github/scripts/pr-intake/tests/pr-intake.test.mjs b/.github/scripts/pr-intake/tests/pr-intake.test.mjs new file mode 100644 index 0000000000..b4ca5467e0 --- /dev/null +++ b/.github/scripts/pr-intake/tests/pr-intake.test.mjs @@ -0,0 +1,662 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + ACTIONS_BOT_ID, + ACTIONS_BOT_LOGIN, + CANONICAL_MARKER, + NEEDS_AUTHOR_FEEDBACK_LABEL, + READY_FOR_REVIEW_LABEL, + ApiError, + buildIntakeReport, + classifyChangedPaths, + deriveVisualAssessment, + determineFeedbackSince, + findClosingIssueReferences, + findVisualEvidence, + getPullRequestWithMergeability, + hasMergeConflict, + isMergeabilityKnown, + parseFeedbackSince, + planManagedLabelChanges, + renderAllClearComment, + renderIntakeComment, + runPullRequestIntake, + selectCanonicalComment, + upsertCanonicalComment, + verifyClosingIssueReferences, +} from '../pr-intake.mjs'; + +function botComment(id, body) { + return { + id, + body, + user: { + login: ACTIONS_BOT_LOGIN, + id: ACTIONS_BOT_ID, + type: 'Bot', + }, + }; +} + +class MockApi { + constructor({ + comments = [], + issues = [], + pullRequests = [], + files = [], + pullRequestQueue = null, + } = {}) { + this.comments = comments; + this.issues = issues; + this.pullRequests = pullRequests; + this.files = files; + this.pullRequestQueue = pullRequestQueue; + this.created = 0; + this.updated = 0; + this.deleted = []; + this.addedLabels = []; + this.removedLabels = []; + this.getIssueCalls = 0; + this.getPullRequestCalls = 0; + this.nextCommentId = 1000; + } + + async listIssueComments(_issueNumber, page) { + return page === 1 ? this.comments : []; + } + + async createIssueComment(_issueNumber, body) { + this.created += 1; + const comment = botComment(this.nextCommentId, body); + this.nextCommentId += 1; + this.comments.push(comment); + return comment; + } + + async updateIssueComment(commentId, body) { + this.updated += 1; + const comment = this.comments.find((entry) => entry.id === commentId); + comment.body = body; + return comment; + } + + async deleteIssueComment(commentId) { + this.deleted.push(commentId); + this.comments = this.comments.filter((entry) => entry.id !== commentId); + return null; + } + + async getIssue(issueNumber) { + this.getIssueCalls += 1; + const issue = this.issues.find((entry) => entry.number === issueNumber); + if (!issue) { + throw new ApiError('Not Found', 404); + } + return issue; + } + + async getPullRequest(issueNumber) { + this.getPullRequestCalls += 1; + if (Array.isArray(this.pullRequestQueue) && this.pullRequestQueue.length) { + return this.pullRequestQueue.shift(); + } + return this.pullRequests.find((entry) => entry.number === issueNumber) + ?? { number: issueNumber, draft: false }; + } + + async listPullRequestFiles(_pullNumber, page) { + return page === 1 ? this.files : []; + } + + async addLabels(issueNumber, labels) { + this.addedLabels.push({ issueNumber, labels }); + return null; + } + + async removeLabel(issueNumber, label) { + this.removedLabels.push({ issueNumber, label }); + return null; + } +} + +test('changed paths provide a visual hint for product UI files', () => { + const report = classifyChangedPaths([ + 'src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml', + 'doc/devdocs/core/architecture.md', + ]); + + assert.equal(report.requiresVisualEvidence, true); + assert.deepEqual(report.visualCandidatePaths, [ + 'src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml', + ]); +}); + +test('docs-only changes do not require visual evidence', () => { + const report = classifyChangedPaths([ + 'README.md', + 'doc/devdocs/core/architecture.md', + ]); + + assert.equal(report.requiresVisualEvidence, false); + assert.deepEqual(report.categories, ['docs']); +}); + +test('deriveVisualAssessment maps the path hint to a requirement', () => { + assert.deepEqual(deriveVisualAssessment(true).visualEvidenceRequirement, 'REQUIRED'); + assert.deepEqual(deriveVisualAssessment(false).visualEvidenceRequirement, 'NOT_NEEDED'); + assert.match(deriveVisualAssessment(true).visualEvidenceReason, /product UI/); +}); + +test('merge conflicts are detected from GitHub mergeability fields', () => { + assert.equal(hasMergeConflict({ mergeable: false }), true); + assert.equal(hasMergeConflict({ mergeable_state: 'dirty' }), true); + assert.equal(hasMergeConflict({ mergeStateStatus: 'CONFLICTING' }), true); + assert.equal(hasMergeConflict({ mergeable: true, mergeable_state: 'clean' }), false); + assert.equal(hasMergeConflict({ mergeable: null, mergeable_state: 'unknown' }), false); +}); + +test('mergeability is only known once GitHub reports a definite state', () => { + assert.equal(isMergeabilityKnown({ mergeable: true }), true); + assert.equal(isMergeabilityKnown({ mergeable: false }), true); + assert.equal(isMergeabilityKnown({ mergeable_state: 'clean' }), true); + assert.equal(isMergeabilityKnown({ mergeable: null, mergeable_state: 'unknown' }), false); + assert.equal(isMergeabilityKnown({ mergeable: null }), false); +}); + +test('getPullRequestWithMergeability retries while mergeability is unknown', async () => { + const api = new MockApi({ + pullRequestQueue: [ + { number: 7, mergeable: null, mergeable_state: 'unknown' }, + { number: 7, mergeable: null, mergeable_state: 'unknown' }, + { number: 7, mergeable: true, mergeable_state: 'clean' }, + ], + }); + + const result = await getPullRequestWithMergeability(api, 7, { delayMs: 0 }); + assert.equal(result.mergeabilityKnown, true); + assert.equal(result.pullRequest.mergeable, true); + assert.equal(api.getPullRequestCalls, 3); +}); + +test('getPullRequestWithMergeability gives up after the attempt cap', async () => { + const api = new MockApi({ + pullRequestQueue: [ + { number: 7, mergeable: null, mergeable_state: 'unknown' }, + { number: 7, mergeable: null, mergeable_state: 'unknown' }, + ], + }); + + const result = await getPullRequestWithMergeability(api, 7, { + maxAttempts: 2, + delayMs: 0, + }); + assert.equal(result.mergeabilityKnown, false); + assert.equal(api.getPullRequestCalls, 2); +}); + +test('closing issue parsing finds supported keywords and de-duplicates issue numbers', () => { + const references = findClosingIssueReferences( + 'Fixes #12\nResolved: #12\nCloses owner/repo#44', + ); + + assert.deepEqual(references, [ + { keyword: 'fixes', issueNumber: 12, repositoryFullName: null }, + { keyword: 'closes', issueNumber: 44, repositoryFullName: 'owner/repo' }, + ]); +}); + +test('closing issue parsing caps the number of references it accepts', () => { + const body = Array.from({ length: 50 }, (_unused, index) => `Closes #${index + 1}`).join('\n'); + const references = findClosingIssueReferences(body); + assert.equal(references.length, 20); +}); + +test('closing issue verification is bounded and preserves reference order', async () => { + const api = new MockApi({ + issues: Array.from({ length: 20 }, (_unused, index) => ({ + number: index + 1, + title: `Issue ${index + 1}`, + })), + }); + const references = Array.from({ length: 20 }, (_unused, index) => ({ + keyword: 'closes', + issueNumber: index + 1, + repositoryFullName: null, + })); + + const result = await verifyClosingIssueReferences({ + api, + repositoryFullName: 'microsoft/PowerToys', + references, + }); + + assert.equal(result.validReferences.length, 20); + assert.deepEqual( + result.validReferences.map((entry) => entry.issueNumber), + references.map((entry) => entry.issueNumber), + ); +}); + +test('closing issue verification accepts local issues and rejects pull requests, other repos, and 404s', async () => { + const api = new MockApi({ + issues: [ + { number: 12, title: 'Tracked bug' }, + { number: 55, title: 'Feature PR', pull_request: { url: 'https://example.test/pr/55' } }, + ], + }); + + const result = await verifyClosingIssueReferences({ + api, + repositoryFullName: 'microsoft/PowerToys', + references: [ + { keyword: 'closes', issueNumber: 12, repositoryFullName: null }, + { keyword: 'fixes', issueNumber: 55, repositoryFullName: null }, + { keyword: 'resolves', issueNumber: 99, repositoryFullName: null }, + { keyword: 'closes', issueNumber: 44, repositoryFullName: 'other/repo' }, + ], + }); + + assert.deepEqual(result.validReferences, [ + { keyword: 'closes', issueNumber: 12, repositoryFullName: null, title: 'Tracked bug' }, + ]); + assert.deepEqual(result.invalidReferences, [ + { keyword: 'fixes', issueNumber: 55, repositoryFullName: null, reason: 'pull-request' }, + { keyword: 'resolves', issueNumber: 99, repositoryFullName: null, reason: 'not-found' }, + { keyword: 'closes', issueNumber: 44, repositoryFullName: 'other/repo', reason: 'different-repository' }, + ]); +}); + +test('visual evidence detection recognizes markdown, attachments, and video links', () => { + const evidence = findVisualEvidence(` +![Screenshot](https://example.com/screenshot.png) +https://github.com/user-attachments/assets/12345678-1234-1234-1234-123456789abc +https://www.youtube.com/watch?v=demo123 +`); + + assert.equal(evidence.found, true); + assert.deepEqual(evidence.types, [ + 'GitHub user-attachment URL', + 'Markdown image', + 'Recognized video link', + ]); +}); + +test('non-visual GitHub file attachments do not satisfy visual evidence', () => { + const evidence = findVisualEvidence(` +https://github.com/user-attachments/files/1234/PowerToysReport_demo.zip +![fake](https://github.com/user-attachments/files/1234/PowerToysReport_demo.zip) + +`); + + assert.equal(evidence.found, false); + assert.deepEqual(evidence.types, []); +}); + +test('label plan removes only managed lifecycle labels', () => { + const plan = planManagedLabelChanges( + ['Product-FancyZones', NEEDS_AUTHOR_FEEDBACK_LABEL], + [READY_FOR_REVIEW_LABEL], + [READY_FOR_REVIEW_LABEL, NEEDS_AUTHOR_FEEDBACK_LABEL], + ); + + assert.deepEqual(plan, { + add: [READY_FOR_REVIEW_LABEL], + remove: [NEEDS_AUTHOR_FEEDBACK_LABEL], + }); +}); + +test('incomplete comment mentions the author and shows only actionable bullets', () => { + const report = buildIntakeReport({ + changedPaths: ['src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml'], + body: '', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + verifiedClosingIssues: [], + invalidClosingIssues: [ + { keyword: 'closes', issueNumber: 999, repositoryFullName: null, reason: 'not-found' }, + ], + }); + + const body = renderIntakeComment(report, '2026-08-05T10:00:00.000Z'); + assert.match(body, /^/); + assert.match(body, /## 🧭 PR intake/); + assert.match(body, /Visual evidence:\*\* Required/); + assert.match(body, /@alice, please update/); + assert.match(body, /invalid closing reference `#999` \(not found\)/); + assert.match(body, /Closes #123/); + assert.match(body, /Replace the invalid closing reference/); + assert.match(body, /screenshot, GIF, or video/); + assert.match(body, /no author response within 7 days/); + assert.equal(parseFeedbackSince(body), '2026-08-05T10:00:00.000Z'); + assert.match(body, /\[contribution guide\]\(https:\/\/github\.com\/microsoft\/PowerToys\/blob\/main\/CONTRIBUTING\.md\)/); + assert.doesNotMatch(body, /Summary:|Ownership matches|Managed labels|Routing|Files scanned/); +}); + +test('complete intake renders the ready state without a summary line', () => { + const report = buildIntakeReport({ + changedPaths: ['doc/devdocs/core/architecture.md'], + body: 'Closes #12', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + verifiedClosingIssues: [{ issueNumber: 12, title: 'Tracked issue' }], + invalidClosingIssues: [], + }); + + const body = renderIntakeComment(report); + assert.equal(report.readyForReview, true); + assert.match(body, /## ✅ Ready for review/); + assert.match(body, /Visual evidence:\*\* Not needed/); + assert.doesNotMatch(body, /Summary:|@alice|contribution guide|Products|Routing/); +}); + +test('missing issue link is recommended without blocking readiness', () => { + const report = buildIntakeReport({ + changedPaths: ['doc/devdocs/core/architecture.md'], + body: 'Clarifies the contributor checklist.', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + verifiedClosingIssues: [], + invalidClosingIssues: [], + }); + + assert.equal(report.readyForReview, true); + assert.equal(report.needsAuthorFeedback, false); + assert.deepEqual(report.authorActions, []); + assert.deepEqual(report.recommendations, [ + 'Link the issue this PR fixes using a closing keyword such as `Closes #123`.', + ]); + assert.match(renderIntakeComment(report), /## Recommendation/); +}); + +test('merge conflict blocks readiness with an author action', () => { + const report = buildIntakeReport({ + changedPaths: ['.github/workflows/issue-triage.md'], + body: 'Closes #12', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + mergeConflict: true, + verifiedClosingIssues: [{ issueNumber: 12, title: 'Tracked issue' }], + invalidClosingIssues: [], + }); + + assert.equal(report.readyForReview, false); + assert.equal(report.needsAuthorFeedback, true); + assert.deepEqual(report.authorActions, [ + 'Resolve the merge conflicts with the target branch.', + ]); + assert.match(renderIntakeComment(report), /Resolve the merge conflicts/); +}); + +test('unknown mergeability holds readiness even when nothing else is flagged', () => { + const report = buildIntakeReport({ + changedPaths: ['doc/devdocs/core/architecture.md'], + body: 'Closes #12', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + mergeabilityKnown: false, + verifiedClosingIssues: [{ issueNumber: 12, title: 'Tracked issue' }], + invalidClosingIssues: [], + }); + + assert.equal(report.readyForReview, false); + assert.equal(report.needsAuthorFeedback, false); + assert.deepEqual(report.authorActions, []); +}); + +test('product UI paths require visual evidence when none is present', () => { + const report = buildIntakeReport({ + changedPaths: ['src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml'], + body: 'Closes #12', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + verifiedClosingIssues: [{ issueNumber: 12, title: 'Tracked issue' }], + invalidClosingIssues: [], + }); + + assert.equal(report.requiresVisualEvidence, true); + assert.equal(report.visualAssessment.visualEvidenceRequirement, 'REQUIRED'); + assert.equal(report.readyForReview, false); + assert.match(report.authorActions.join(' '), /screenshot, GIF, or video/); +}); + +test('product UI paths are satisfied when visual evidence is present', () => { + const report = buildIntakeReport({ + changedPaths: ['src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml'], + body: 'Closes #12\n![screenshot](https://example.com/shot.png)', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + verifiedClosingIssues: [{ issueNumber: 12, title: 'Tracked issue' }], + invalidClosingIssues: [], + }); + + assert.equal(report.requiresVisualEvidence, true); + assert.equal(report.readyForReview, true); + assert.doesNotMatch(report.authorActions.join(' '), /screenshot/i); +}); + +test('draft PR remains incomplete until marked ready', () => { + const report = buildIntakeReport({ + changedPaths: ['doc/devdocs/core/architecture.md'], + body: 'Closes #12', + repositoryHtmlUrl: 'https://github.com/microsoft/PowerToys', + baseRef: 'main', + authorLogin: 'alice', + isDraft: true, + verifiedClosingIssues: [{ issueNumber: 12, title: 'Tracked issue' }], + invalidClosingIssues: [], + }); + + assert.equal(report.readyForReview, false); + assert.equal(report.needsAuthorFeedback, false); + assert.deepEqual(report.authorActions, ['Mark the pull request as ready for review.']); +}); + +test('readiness and author-feedback labels are mutually managed', () => { + assert.deepEqual( + planManagedLabelChanges( + ['Product-Keyboard Manager'], + [READY_FOR_REVIEW_LABEL], + [READY_FOR_REVIEW_LABEL, NEEDS_AUTHOR_FEEDBACK_LABEL], + ), + { add: [READY_FOR_REVIEW_LABEL], remove: [] }, + ); + assert.deepEqual( + planManagedLabelChanges( + ['Product-Keyboard Manager', READY_FOR_REVIEW_LABEL], + [], + [READY_FOR_REVIEW_LABEL, NEEDS_AUTHOR_FEEDBACK_LABEL], + ), + { add: [], remove: [READY_FOR_REVIEW_LABEL] }, + ); + assert.deepEqual( + planManagedLabelChanges( + ['Product-FancyZones', READY_FOR_REVIEW_LABEL], + [NEEDS_AUTHOR_FEEDBACK_LABEL], + [READY_FOR_REVIEW_LABEL, NEEDS_AUTHOR_FEEDBACK_LABEL], + ), + { add: [NEEDS_AUTHOR_FEEDBACK_LABEL], remove: [READY_FOR_REVIEW_LABEL] }, + ); +}); + +test('author activity resets the feedback window while bot activity does not', () => { + const existingBody = renderIntakeComment( + { + readyForReview: false, + needsAuthorFeedback: true, + authorLogin: 'alice', + authorActions: ['Update the PR.'], + recommendations: [], + contributingUrl: 'https://example.test/CONTRIBUTING.md', + visualEvidence: { found: false, types: [] }, + visualAssessment: { + visualEvidenceRequirement: 'NOT_NEEDED', + visualEvidenceReason: 'The change is nonvisual.', + }, + }, + '2026-08-01T10:00:00.000Z', + ); + + assert.equal( + determineFeedbackSince({ + needsAuthorFeedback: true, + existingCommentBody: existingBody, + action: 'edited', + senderLogin: 'alice', + authorLogin: 'alice', + now: '2026-08-05T10:00:00.000Z', + }), + '2026-08-05T10:00:00.000Z', + ); + assert.equal( + determineFeedbackSince({ + needsAuthorFeedback: true, + existingCommentBody: existingBody, + action: 'edited', + senderLogin: ACTIONS_BOT_LOGIN, + authorLogin: 'alice', + now: '2026-08-05T10:00:00.000Z', + }), + '2026-08-01T10:00:00.000Z', + ); +}); + +test('canonical selection trusts only the GitHub Actions bot and chooses the oldest marker', () => { + const body = `${CANONICAL_MARKER}\ncomment`; + const selected = selectCanonicalComment([ + botComment(30, body), + { + id: 10, + body, + user: { login: ACTIONS_BOT_LOGIN, id: 99, type: 'Bot' }, + }, + { + id: 5, + body, + user: { login: 'attacker', id: 1, type: 'User' }, + }, + botComment(20, body), + ]); + + assert.equal(selected.canonical.id, 20); + assert.deepEqual(selected.extras.map((entry) => entry.id), [30]); +}); + +test('canonical upsert updates the oldest trusted comment and deletes extras', async () => { + const body = `${CANONICAL_MARKER}\nfirst`; + const api = new MockApi({ + comments: [botComment(20, body), botComment(40, body)], + }); + + const result = await upsertCanonicalComment({ + api, + issueNumber: 12, + body: `${CANONICAL_MARKER}\nupdated`, + }); + + assert.equal(result.operation, 'updated'); + assert.equal(result.comment.id, 20); + assert.deepEqual(result.deletedExtraComments, [40]); + assert.equal(api.created, 0); + assert.equal(api.updated, 1); + assert.deepEqual(api.deleted, [40]); + assert.equal(api.comments.length, 1); +}); + +test('all-clear comment carries the canonical marker', () => { + const body = renderAllClearComment(); + assert.match(body, /^/); + assert.match(body, /All automated intake checks now pass/); +}); + +function intakeEvent(overrides = {}) { + return { + action: 'opened', + repository: { + full_name: 'microsoft/PowerToys', + html_url: 'https://github.com/microsoft/PowerToys', + }, + pull_request: { number: 100 }, + sender: { login: 'alice' }, + ...overrides, + }; +} + +test('runPullRequestIntake stays silent on a clean PR with no prior comment', async () => { + const api = new MockApi({ + issues: [{ number: 100, labels: [], title: 'PR' }], + pullRequests: [{ + number: 100, + draft: false, + mergeable: true, + mergeable_state: 'clean', + body: 'Closes #12', + base: { ref: 'main' }, + user: { login: 'alice' }, + }], + files: [{ filename: 'doc/devdocs/core/architecture.md', status: 'modified' }], + }); + api.issues.push({ number: 12, title: 'Tracked issue' }); + + const result = await runPullRequestIntake({ api, event: intakeEvent() }); + assert.equal(result.commentResult.operation, 'skipped'); + assert.equal(api.created, 0); + assert.equal(api.updated, 0); + assert.deepEqual(result.labelPlan.add, [READY_FOR_REVIEW_LABEL]); +}); + +test('runPullRequestIntake replaces a stale comment with an all-clear note when the PR is clean', async () => { + const existing = botComment(50, `${CANONICAL_MARKER}\n## 🧭 PR intake\nplease update`); + const api = new MockApi({ + comments: [existing], + issues: [{ number: 100, labels: [NEEDS_AUTHOR_FEEDBACK_LABEL], title: 'PR' }], + pullRequests: [{ + number: 100, + draft: false, + mergeable: true, + mergeable_state: 'clean', + body: 'Closes #12', + base: { ref: 'main' }, + user: { login: 'alice' }, + }], + files: [{ filename: 'doc/devdocs/core/architecture.md', status: 'modified' }], + }); + api.issues.push({ number: 12, title: 'Tracked issue' }); + + const result = await runPullRequestIntake({ api, event: intakeEvent() }); + assert.equal(result.commentResult.operation, 'updated'); + assert.match(existing.body, /All automated intake checks now pass/); + assert.deepEqual(result.labelPlan.remove, [NEEDS_AUTHOR_FEEDBACK_LABEL]); +}); + +test('runPullRequestIntake posts a feedback comment when there are remarks', async () => { + const api = new MockApi({ + issues: [{ number: 100, labels: [], title: 'PR' }], + pullRequests: [{ + number: 100, + draft: false, + mergeable: true, + mergeable_state: 'clean', + body: 'No linked issue here.', + base: { ref: 'main' }, + user: { login: 'alice' }, + }], + files: [{ + filename: 'src/settings-ui/Settings.UI/SettingsXAML/Views/DashboardPage.xaml', + status: 'modified', + }], + }); + + const result = await runPullRequestIntake({ api, event: intakeEvent() }); + assert.equal(result.commentResult.operation, 'created'); + assert.equal(api.created, 1); + assert.equal(result.readyForReview, false); + assert.match(api.comments[0].body, /screenshot, GIF, or video/); +}); diff --git a/.github/workflows/pr-intake.yml b/.github/workflows/pr-intake.yml new file mode 100644 index 0000000000..0f14a39204 --- /dev/null +++ b/.github/workflows/pr-intake.yml @@ -0,0 +1,51 @@ +# PR intake +# +# Deterministic pull request intake. Runs on every pull request and posts a +# single canonical comment (only when there is something for the author to act +# on or consider), keeps the Needs-Review / Needs-Author-Feedback lifecycle +# labels in sync, and nudges for visual evidence on product UI changes. +# +# This workflow does not run any code from the pull request head. The Node +# script reads all pull request data through the GitHub API, so checking out +# the base ref is only needed to run the script itself. 3rd-party actions are +# pinned to a commit hash per Microsoft's security guidelines. +name: PR intake + +on: + pull_request_target: + types: + - opened + - edited + - synchronize + - reopened + - ready_for_review + - converted_to_draft + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: pr-intake-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + pr-intake: + runs-on: ubuntu-latest + steps: + - name: Checkout base ref + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: "22" + + - name: Run PR intake + env: + GITHUB_TOKEN: ${{ github.token }} + run: node .github/scripts/pr-intake/pr-intake.mjs "$GITHUB_EVENT_PATH"