|
| 1 | +// Review/approval integrity signals, read from structured PR-reviews data only — no diff/text/YAML parsing. |
| 2 | +// Surfaces cases a PR's own page does not always make obvious without branch protection's "dismiss stale reviews" |
| 3 | +// setting enabled: an APPROVED review that predates the current head commit (new pushes landed since the |
| 4 | +// approval), the PR author approving their own PR, and a reviewer whose CURRENT (most recent) review is still |
| 5 | +// CHANGES_REQUESTED. Reads only documented fields from the GitHub PR-reviews API (state, commit_id, user.login, |
| 6 | +// submitted_at) and compares them — no ambiguous-syntax parsing, so it cannot suffer a patch scanner's edge cases. |
| 7 | +// Pure GitHub-metadata read, no repo content. Fail-safe: no token, no head SHA, a bad repo slug, or a fetch error |
| 8 | +// all yield no finding. Bounded to MAX_PAGES pages of reviews (REVIEWS_PER_PAGE each) — a pathological PR can't |
| 9 | +// spin the analyzer, but any realistic PR's full review history is read, not just its oldest page. |
| 10 | +import type { |
| 11 | + AnalyzerDiagnostics, |
| 12 | + ApprovalIntegrityFinding, |
| 13 | + EnrichRequest, |
| 14 | +} from "../types.js"; |
| 15 | +import type { AnalysisContext } from "../analysis-context.js"; |
| 16 | +import { boundedFetchJson } from "../external-fetch.js"; |
| 17 | + |
| 18 | +const GITHUB_API = "https://api.github.com"; |
| 19 | +const SLUG_RE = /^[A-Za-z0-9._-]+$/; |
| 20 | +const REVIEWS_PER_PAGE = 100; |
| 21 | +// GitHub returns PR reviews oldest-first with no reorder option, so a single `per_page=100` fetch would silently |
| 22 | +// read only the OLDEST reviews on any PR with more — exactly backwards for "each reviewer's latest vote". Walk |
| 23 | +// pages instead, bounded so a pathological PR can't spin (mirrors this repo's own PR_DETAIL_MAX_PAGES convention). |
| 24 | +const MAX_PAGES = 10; |
| 25 | +const SHA_PREFIX_LEN = 12; |
| 26 | + |
| 27 | +interface ScanOptions { |
| 28 | + signal?: AbortSignal; |
| 29 | + analysis?: Pick<AnalysisContext, "fetchJson">; |
| 30 | + diagnostics?: AnalyzerDiagnostics; |
| 31 | +} |
| 32 | + |
| 33 | +/** The slice of a GitHub PR-review list item this analyzer reads. */ |
| 34 | +interface ReviewListItem { |
| 35 | + user?: { login?: string } | null; |
| 36 | + state?: string; |
| 37 | + commit_id?: string; |
| 38 | + submitted_at?: string | null; |
| 39 | +} |
| 40 | + |
| 41 | +/** One reviewer's current (most recent submitted) vote. */ |
| 42 | +interface LatestReview { |
| 43 | + login: string; |
| 44 | + state: string; |
| 45 | + commitId: string | undefined; |
| 46 | + submittedAt: string; |
| 47 | +} |
| 48 | + |
| 49 | +function githubHeaders(token: string): Record<string, string> { |
| 50 | + return { |
| 51 | + Authorization: `Bearer ${token}`, |
| 52 | + Accept: "application/vnd.github+json", |
| 53 | + "X-GitHub-Api-Version": "2022-11-28", |
| 54 | + }; |
| 55 | +} |
| 56 | + |
| 57 | +async function fetchReviewsPage( |
| 58 | + owner: string, |
| 59 | + repo: string, |
| 60 | + prNumber: number, |
| 61 | + page: number, |
| 62 | + headers: Record<string, string>, |
| 63 | + fetchFn: typeof fetch, |
| 64 | + signal: AbortSignal | undefined, |
| 65 | + options: Pick<ScanOptions, "analysis" | "diagnostics">, |
| 66 | +): Promise<ReviewListItem[] | null> { |
| 67 | + const url = |
| 68 | + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls/` + |
| 69 | + `${encodeURIComponent(String(prNumber))}/reviews?per_page=${REVIEWS_PER_PAGE}&page=${page}`; |
| 70 | + const fetchOptions = { |
| 71 | + endpointCategory: "github-pr-reviews", |
| 72 | + headers, |
| 73 | + signal, |
| 74 | + fetchImpl: fetchFn, |
| 75 | + diagnostics: options.diagnostics, |
| 76 | + phase: "approval-integrity", |
| 77 | + subcall: "github-pr-reviews", |
| 78 | + maxBytes: 512 * 1024, |
| 79 | + }; |
| 80 | + const response = options.analysis |
| 81 | + ? await options.analysis.fetchJson<ReviewListItem[]>(url, fetchOptions) |
| 82 | + : await boundedFetchJson<ReviewListItem[]>(url, fetchOptions); |
| 83 | + return response.ok && Array.isArray(response.data) ? response.data : null; |
| 84 | +} |
| 85 | + |
| 86 | +/** Walks review pages (oldest-first, as GitHub returns them) up to MAX_PAGES, so `latestReviewPerReviewer` sees |
| 87 | + * every reviewer's true latest vote rather than just the oldest page. A short page (fewer than REVIEWS_PER_PAGE |
| 88 | + * items) confirms there is nothing further — no Link-header parsing needed — and only THEN is the list returned. |
| 89 | + * Unlike a purely additive signal (e.g. blame-link's "last touched by"), a PARTIAL oldest-first list here is not |
| 90 | + * a smaller-but-still-correct result — it is actively WRONG: a reviewer's true latest vote could sit on a page |
| 91 | + * never read, making a resolved CHANGES_REQUESTED or a fresh APPROVED look stuck/stale. So any page failure, OR |
| 92 | + * exhausting MAX_PAGES while the last page fetched was still full (pagination not confirmed complete), fails |
| 93 | + * the whole call closed (null) rather than treating incomplete history as authoritative. */ |
| 94 | +async function fetchReviews( |
| 95 | + owner: string, |
| 96 | + repo: string, |
| 97 | + prNumber: number, |
| 98 | + headers: Record<string, string>, |
| 99 | + fetchFn: typeof fetch, |
| 100 | + signal: AbortSignal | undefined, |
| 101 | + options: Pick<ScanOptions, "analysis" | "diagnostics">, |
| 102 | +): Promise<ReviewListItem[] | null> { |
| 103 | + const items: ReviewListItem[] = []; |
| 104 | + for (let page = 1; page <= MAX_PAGES; page += 1) { |
| 105 | + const pageItems = await fetchReviewsPage(owner, repo, prNumber, page, headers, fetchFn, signal, options); |
| 106 | + if (!pageItems) return null; // any page failing means we cannot trust what we have so far |
| 107 | + items.push(...pageItems); |
| 108 | + if (pageItems.length < REVIEWS_PER_PAGE) return items; // confirmed: this was the last page |
| 109 | + } |
| 110 | + return null; // MAX_PAGES exhausted with a still-full final page — completeness unconfirmed |
| 111 | +} |
| 112 | + |
| 113 | +/** Reduces a PR's review list to one entry per reviewer: the review with the latest `submitted_at`. This mirrors |
| 114 | + * GitHub's own semantics for "a reviewer's current vote" — a later review of ANY state supersedes an earlier one |
| 115 | + * from the same person, including a dismissal (the API reports a dismissed review back with `state: "DISMISSED"`, |
| 116 | + * so a dismissed CHANGES_REQUESTED naturally stops counting as outstanding without any extra handling here). |
| 117 | + * Reviews with no `submitted_at` (a still-open PENDING draft review) are excluded — not yet a submitted vote. |
| 118 | + * Login comparison is case-insensitive (GitHub logins are case-insensitive), keyed on the lowercased login. |
| 119 | + * `reviews` must be in GitHub's own (oldest-first) API order: on a `submitted_at` tie (same timestamp |
| 120 | + * precision), the LATER item in that order wins, using API order itself as the tie-break. Pure. */ |
| 121 | +export function latestReviewPerReviewer(reviews: ReviewListItem[]): Map<string, LatestReview> { |
| 122 | + const latest = new Map<string, LatestReview>(); |
| 123 | + for (const review of reviews) { |
| 124 | + const login = review.user?.login; |
| 125 | + const state = review.state; |
| 126 | + const submittedAt = review.submitted_at; |
| 127 | + if (!login || !state || !submittedAt) continue; |
| 128 | + const key = login.toLowerCase(); |
| 129 | + const existing = latest.get(key); |
| 130 | + if (!existing || submittedAt >= existing.submittedAt) { |
| 131 | + latest.set(key, { login, state, commitId: review.commit_id, submittedAt }); |
| 132 | + } |
| 133 | + } |
| 134 | + return latest; |
| 135 | +} |
| 136 | + |
| 137 | +/** Analyzer entrypoint: a PR's reviews → stale/self/outstanding approval-integrity findings. Fail-safe — no token, |
| 138 | + * no head SHA, a bad repo slug, or a fetch error all yield no finding rather than an error. */ |
| 139 | +export async function scanApprovalIntegrity( |
| 140 | + req: EnrichRequest, |
| 141 | + fetchFn: typeof fetch = fetch, |
| 142 | + options: ScanOptions = {}, |
| 143 | +): Promise<ApprovalIntegrityFinding[]> { |
| 144 | + const { repoFullName, githubToken, headSha, author, prNumber } = req; |
| 145 | + if (!githubToken || !headSha) return []; |
| 146 | + const parts = repoFullName.split("/"); |
| 147 | + const owner = parts[0]; |
| 148 | + const repo = parts[1]; |
| 149 | + if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; |
| 150 | + |
| 151 | + const headers = githubHeaders(githubToken); |
| 152 | + const reviews = await fetchReviews(owner, repo, prNumber, headers, fetchFn, options.signal, options); |
| 153 | + if (!reviews) return []; |
| 154 | + |
| 155 | + const findings: ApprovalIntegrityFinding[] = []; |
| 156 | + const authorKey = author?.toLowerCase(); |
| 157 | + const headShaKey = headSha.toLowerCase(); |
| 158 | + for (const { login, state, commitId } of latestReviewPerReviewer(reviews).values()) { |
| 159 | + if (state === "APPROVED") { |
| 160 | + if (commitId && commitId.toLowerCase() !== headShaKey) { |
| 161 | + findings.push({ |
| 162 | + reviewer: login, |
| 163 | + kind: "stale-approval", |
| 164 | + reviewedShaPrefix: commitId.slice(0, SHA_PREFIX_LEN), |
| 165 | + }); |
| 166 | + } |
| 167 | + if (authorKey && login.toLowerCase() === authorKey) { |
| 168 | + findings.push({ reviewer: login, kind: "self-approval" }); |
| 169 | + } |
| 170 | + } else if (state === "CHANGES_REQUESTED") { |
| 171 | + findings.push({ reviewer: login, kind: "outstanding-changes-requested" }); |
| 172 | + } |
| 173 | + } |
| 174 | + return findings; |
| 175 | +} |
0 commit comments