diff --git a/src/approval-report/__tests__/build-rule-approval-rows.test.ts b/src/approval-report/__tests__/build-rule-approval-rows.test.ts index c536532..653a766 100644 --- a/src/approval-report/__tests__/build-rule-approval-rows.test.ts +++ b/src/approval-report/__tests__/build-rule-approval-rows.test.ts @@ -51,6 +51,7 @@ function mockDeps( getDefinitionPages: () => [], loadApprovalByRuleId: () => ({}), fetchOpenIssues: async () => [], + fetchOpenPublishPrs: async () => [], getRuleDefinitions: () => [], getChangesSinceApproval: () => [], getLatestCommitDateOnPaths: () => "2024-01-01", @@ -167,6 +168,27 @@ describe("buildRuleApprovalRows", () => { expect(rows[0].reviewPrUrl).toBeNull(); }); + it("sets reviewPrUrl and In review from an open publication PR", async () => { + const fetchOpenPublishPrs = jest.fn(async () => [ + { + number: 42, + html_url: "https://github.com/w3c/wcag-act-rules/pull/42", + ruleIds: ["674b10"], + }, + ]); + const rows = await buildRuleApprovalRows( + baseOpts, + oneAtomic("674b10", { fetchOpenPublishPrs }), + ); + + expect(fetchOpenPublishPrs).toHaveBeenCalledWith("w3c", "wcag-act-rules"); + expect(rows[0]).toMatchObject({ + reviewPrUrl: "https://github.com/w3c/wcag-act-rules/pull/42", + status: "In review", + reportBucket: "notReady", + }); + }); + it("buckets approvedUpToDate when approved with no commits after approval", async () => { const rows = await buildRuleApprovalRows( baseOpts, diff --git a/src/approval-report/__tests__/github-publish-prs.test.ts b/src/approval-report/__tests__/github-publish-prs.test.ts new file mode 100644 index 0000000..59f2e2b --- /dev/null +++ b/src/approval-report/__tests__/github-publish-prs.test.ts @@ -0,0 +1,231 @@ +jest.mock("@octokit/rest", () => ({ + Octokit: jest.fn(), +})); + +import { Octokit } from "@octokit/rest"; +import { + fetchOpenPublishPrs, + reviewPrUrlsByRuleId, +} from "../github-publish-prs"; + +describe("fetchOpenPublishPrs", () => { + const list = jest.fn(); + const listFiles = jest.fn(); + const iterator = jest.fn(); + const originalGithubToken = process.env.GITHUB_TOKEN; + + beforeEach(() => { + jest.clearAllMocks(); + delete process.env.GITHUB_TOKEN; + (Octokit as unknown as jest.Mock).mockImplementation(() => ({ + paginate: { iterator }, + rest: { pulls: { list, listFiles } }, + })); + }); + + afterAll(() => { + if (originalGithubToken === undefined) { + delete process.env.GITHUB_TOKEN; + } else { + process.env.GITHUB_TOKEN = originalGithubToken; + } + }); + + function mockGitHub( + pulls: Array<{ number: number; html_url: string }>, + filesByPull: Record, + ): void { + iterator.mockImplementation( + ( + endpoint: unknown, + options: { pull_number?: number }, + ): AsyncGenerator<{ data: unknown[] }> => { + async function* responses() { + if (endpoint === list) { + yield { data: pulls }; + } else { + yield { + data: (filesByPull[options.pull_number ?? -1] ?? []).map( + (filename) => ({ filename }), + ), + }; + } + } + return responses(); + }, + ); + } + + it("matches only content/rules/{sixCharId}/index.md paths", async () => { + mockGitHub([{ number: 12, html_url: "https://example.test/pull/12" }], { + 12: [ + "content/rules/674b10/index.md", + "content/rules/674b10/example.md", + "content/rules/not-six/index.md", + "content/glossary/example.md", + ], + }); + + await expect(fetchOpenPublishPrs("w3c", "wcag-act-rules")).resolves.toEqual( + [ + { + number: 12, + html_url: "https://example.test/pull/12", + ruleIds: ["674b10"], + }, + ], + ); + expect(iterator).toHaveBeenCalledWith( + list, + expect.objectContaining({ + owner: "w3c", + repo: "wcag-act-rules", + state: "open", + }), + ); + expect(Octokit).toHaveBeenCalledWith({ auth: undefined }); + }); + + it("lists publication PRs with state open", async () => { + mockGitHub([], {}); + + await fetchOpenPublishPrs("w3c", "wcag-act-rules"); + + expect(iterator).toHaveBeenCalledWith( + list, + expect.objectContaining({ + owner: "w3c", + repo: "wcag-act-rules", + state: "open", + }), + ); + }); + + it("returns no match when no open PR is listed", async () => { + mockGitHub([], {}); + + await expect(fetchOpenPublishPrs("w3c", "wcag-act-rules")).resolves.toEqual( + [], + ); + expect(listFiles).not.toHaveBeenCalled(); + }); + + it("maps every rule index changed by one PR", async () => { + mockGitHub([{ number: 20, html_url: "https://example.test/pull/20" }], { + 20: ["content/rules/674b10/index.md", "content/rules/a1b2c3/index.md"], + }); + + const prs = await fetchOpenPublishPrs("w3c", "wcag-act-rules"); + expect(prs[0].ruleIds).toEqual(["674b10", "a1b2c3"]); + }); + + it("uses GITHUB_TOKEN when one is available", async () => { + process.env.GITHUB_TOKEN = "test-token"; + mockGitHub([], {}); + + await fetchOpenPublishPrs("w3c", "wcag-act-rules"); + + expect(Octokit).toHaveBeenCalledWith({ auth: "test-token" }); + }); + + it.each([ + { status: 404, message: "Not Found" }, + { status: 403, message: "Forbidden" }, + ])( + "retries without auth on HTTP $status when an installation token cannot access W3C", + async ({ status, message }) => { + process.env.GITHUB_TOKEN = "installation-token"; + iterator + .mockImplementationOnce(() => { + async function* denied() { + throw { status, message }; + yield { data: [] }; + } + return denied(); + }) + .mockImplementationOnce(() => { + async function* publicResponse() { + yield { data: [] }; + } + return publicResponse(); + }); + + await expect( + fetchOpenPublishPrs("w3c", "wcag-act-rules"), + ).resolves.toEqual([]); + expect(Octokit).toHaveBeenNthCalledWith(1, { + auth: "installation-token", + }); + expect(Octokit).toHaveBeenNthCalledWith(2, { auth: undefined }); + }, + ); + + it("reports repository and rate-limit failures clearly", async () => { + iterator.mockImplementation(() => { + async function* responses() { + throw { + status: 403, + message: "API rate limit exceeded", + response: { + headers: { + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": "123", + }, + }, + }; + yield { data: [] }; + } + return responses(); + }); + + await expect(fetchOpenPublishPrs("w3c", "wcag-act-rules")).rejects.toThrow( + /Unable to list open publication PRs for w3c\/wcag-act-rules.*rate limit.*GITHUB_TOKEN/i, + ); + }); + + it("does not retry unauthenticated when rate-limited with GITHUB_TOKEN", async () => { + process.env.GITHUB_TOKEN = "installation-token"; + iterator.mockImplementation(() => { + async function* responses() { + throw { + status: 403, + message: "API rate limit exceeded", + response: { + headers: { + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": "123", + }, + }, + }; + yield { data: [] }; + } + return responses(); + }); + + await expect(fetchOpenPublishPrs("w3c", "wcag-act-rules")).rejects.toThrow( + /Unable to list open publication PRs for w3c\/wcag-act-rules.*rate limit.*GITHUB_TOKEN/i, + ); + expect(Octokit).toHaveBeenCalledTimes(1); + expect(Octokit).toHaveBeenCalledWith({ auth: "installation-token" }); + expect(Octokit).not.toHaveBeenCalledWith({ auth: undefined }); + }); +}); + +describe("reviewPrUrlsByRuleId", () => { + it("selects the lowest PR number for a rule deterministically", () => { + const urls = reviewPrUrlsByRuleId([ + { + number: 99, + html_url: "https://example.test/pull/99", + ruleIds: ["674b10"], + }, + { + number: 7, + html_url: "https://example.test/pull/7", + ruleIds: ["674b10"], + }, + ]); + + expect(urls.get("674b10")).toBe("https://example.test/pull/7"); + }); +}); diff --git a/src/approval-report/build-rule-approval-rows.ts b/src/approval-report/build-rule-approval-rows.ts index 938046c..2040485 100644 --- a/src/approval-report/build-rule-approval-rows.ts +++ b/src/approval-report/build-rule-approval-rows.ts @@ -11,6 +11,10 @@ import { issueHasBlockerLabel, issuesForRuleId, } from "./github-issues"; +import { + fetchOpenPublishPrs, + reviewPrUrlsByRuleId, +} from "./github-publish-prs"; import { loadApprovalByRuleId, loadCompleteImplementationsByRuleId, @@ -81,6 +85,7 @@ export type ApprovalReportDeps = { loadApprovalByRuleId: typeof loadApprovalByRuleId; loadCompleteImplementationsByRuleId: typeof loadCompleteImplementationsByRuleId; fetchOpenIssues: typeof fetchOpenIssues; + fetchOpenPublishPrs: typeof fetchOpenPublishPrs; getRuleDefinitions: typeof getRuleDefinitions; getChangesSinceApproval: typeof getChangesSinceApproval; getLatestCommitDateOnPaths: typeof getLatestCommitDateOnPaths; @@ -93,6 +98,7 @@ const defaultDeps: ApprovalReportDeps = { loadApprovalByRuleId, loadCompleteImplementationsByRuleId, fetchOpenIssues, + fetchOpenPublishPrs, getRuleDefinitions, getChangesSinceApproval, getLatestCommitDateOnPaths, @@ -109,6 +115,9 @@ export async function buildRuleApprovalRows( const approvalById = d.loadApprovalByRuleId(opts.wcagActRulesDir); const implById = d.loadCompleteImplementationsByRuleId(opts.wcagActRulesDir); const openIssues = await d.fetchOpenIssues(opts.githubOwner, opts.githubRepo); + const reviewPrUrls = reviewPrUrlsByRuleId( + await d.fetchOpenPublishPrs("w3c", "wcag-act-rules"), + ); const referencedAtomicIds = buildAtomicIdsReferencedByComposites(rules); const rows: RuleApprovalRow[] = []; @@ -157,7 +166,7 @@ export async function buildRuleApprovalRows( const waiApproved = Boolean(approval.approved && approval.approvalIsoDate); const lastApprovedSummary = approval.approvalIsoDate ?? "-"; const commitsBehindSummary = waiApproved ? String(changes.length) : "-"; - const reviewPrUrl = null; + const reviewPrUrl = reviewPrUrls.get(ruleId.toLowerCase()) ?? null; const status = classifyRuleStatus({ deprecated: Boolean(rule.frontmatter.deprecated), reviewPrUrl, diff --git a/src/approval-report/github-publish-prs.ts b/src/approval-report/github-publish-prs.ts new file mode 100644 index 0000000..6a72791 --- /dev/null +++ b/src/approval-report/github-publish-prs.ts @@ -0,0 +1,159 @@ +import { Octokit } from "@octokit/rest"; + +export type OpenPublishPr = { + number: number; + html_url: string; + ruleIds: string[]; +}; + +type PullRequestBatch = Array<{ + number: number; + html_url: string; +}>; + +type PullRequestFileBatch = Array<{ + filename: string; +}>; + +const RULE_INDEX_PATH = /^content\/rules\/([a-z0-9]{6})\/index\.md$/i; + +/** + * List open WAI publication PRs and the ACT rule ids whose index files they + * change. GITHUB_TOKEN is optional because wcag-act-rules is public. + */ +export async function fetchOpenPublishPrs( + owner: string, + repo: string, +): Promise { + const token = process.env.GITHUB_TOKEN; + + try { + return await listOpenPublishPrs( + new Octokit({ auth: token || undefined }), + owner, + repo, + ); + } catch (error) { + if (token && isAccessError(error)) { + try { + return await listOpenPublishPrs( + new Octokit({ auth: undefined }), + owner, + repo, + ); + } catch (fallbackError) { + throwFetchError(owner, repo, fallbackError); + } + } + throwFetchError(owner, repo, error); + } +} + +async function listOpenPublishPrs( + octokit: Octokit, + owner: string, + repo: string, +): Promise { + const pulls: PullRequestBatch = []; + const responses = octokit.paginate.iterator(octokit.rest.pulls.list, { + owner, + repo, + state: "open", + per_page: 100, + }); + + for await (const response of responses) { + pulls.push(...(response.data as PullRequestBatch)); + } + + const publishPrs: OpenPublishPr[] = []; + for (const pull of pulls.sort((a, b) => a.number - b.number)) { + const ruleIds = new Set(); + const fileResponses = octokit.paginate.iterator( + octokit.rest.pulls.listFiles, + { + owner, + repo, + pull_number: pull.number, + per_page: 100, + }, + ); + + for await (const response of fileResponses) { + for (const file of response.data as PullRequestFileBatch) { + const match = RULE_INDEX_PATH.exec(file.filename); + if (match) ruleIds.add(match[1].toLowerCase()); + } + } + + if (ruleIds.size > 0) { + publishPrs.push({ + number: pull.number, + html_url: pull.html_url, + ruleIds: [...ruleIds], + }); + } + } + + return publishPrs; +} + +/** Map each rule to the lowest-numbered open PR that publishes it. */ +export function reviewPrUrlsByRuleId( + prs: OpenPublishPr[], +): Map { + const urls = new Map(); + for (const pr of [...prs].sort((a, b) => a.number - b.number)) { + for (const ruleId of pr.ruleIds) { + const normalizedId = ruleId.toLowerCase(); + if (!urls.has(normalizedId)) urls.set(normalizedId, pr.html_url); + } + } + return urls; +} + +function githubErrorMessage(error: unknown): string { + if (!error || typeof error !== "object") return String(error); + + const githubError = error as { + message?: string; + status?: number; + response?: { + headers?: { + "x-ratelimit-remaining"?: string; + "x-ratelimit-reset"?: string; + }; + }; + }; + const status = githubError.status ? `HTTP ${githubError.status}: ` : ""; + const rateLimited = + githubError.status === 403 && + githubError.response?.headers?.["x-ratelimit-remaining"] === "0"; + const rateLimitHint = rateLimited + ? ` GitHub API rate limit was exceeded${ + githubError.response?.headers?.["x-ratelimit-reset"] + ? ` (reset ${githubError.response.headers["x-ratelimit-reset"]})` + : "" + }; set GITHUB_TOKEN to increase the limit.` + : ""; + return `${status}${githubError.message ?? "Unknown GitHub API error"}.${rateLimitHint}`; +} + +function isAccessError(error: unknown): boolean { + if (!error || typeof error !== "object") return false; + const githubError = error as { + status?: number; + response?: { headers?: { "x-ratelimit-remaining"?: string } }; + }; + if (githubError.status === 404) return true; + if (githubError.status !== 403) return false; + return githubError.response?.headers?.["x-ratelimit-remaining"] !== "0"; +} + +function throwFetchError(owner: string, repo: string, error: unknown): never { + throw new Error( + `Unable to list open publication PRs for ${owner}/${repo}: ${githubErrorMessage( + error, + )}`, + ); +}