From 79bec082221d17e9dadd0fcec11156f7c1d27d8c Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Mon, 21 Sep 2026 16:48:06 +0200 Subject: [PATCH 1/2] feat: map open publish PRs to rule status Fetch open pull requests from w3c/wcag-act-rules and map changed rule index files to In review approval-report rows. Co-authored-by: Cursor --- .../build-rule-approval-rows.test.ts | 22 +++ .../__tests__/github-publish-prs.test.ts | 178 ++++++++++++++++++ .../build-rule-approval-rows.ts | 11 +- src/approval-report/github-publish-prs.ts | 154 +++++++++++++++ 4 files changed, 364 insertions(+), 1 deletion(-) create mode 100644 src/approval-report/__tests__/github-publish-prs.test.ts create mode 100644 src/approval-report/github-publish-prs.ts 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..ab99ca3 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 rows = await buildRuleApprovalRows( + baseOpts, + oneAtomic("674b10", { + fetchOpenPublishPrs: async () => [ + { + number: 42, + html_url: "https://github.com/w3c/wcag-act-rules/pull/42", + ruleIds: ["674b10"], + }, + ], + }), + ); + + 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..e373bfa --- /dev/null +++ b/src/approval-report/__tests__/github-publish-prs.test.ts @@ -0,0 +1,178 @@ +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(); + + beforeEach(() => { + jest.clearAllMocks(); + delete process.env.GITHUB_TOKEN; + (Octokit as unknown as jest.Mock).mockImplementation(() => ({ + paginate: { iterator }, + rest: { pulls: { list, listFiles } }, + })); + }); + + afterAll(() => { + delete process.env.GITHUB_TOKEN; + }); + + 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("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("retries without auth when an installation token cannot access W3C", async () => { + process.env.GITHUB_TOKEN = "installation-token"; + iterator + .mockImplementationOnce(() => { + async function* denied() { + throw { status: 404, message: "Not Found" }; + 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, + ); + }); +}); + +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..837126b --- /dev/null +++ b/src/approval-report/github-publish-prs.ts @@ -0,0 +1,154 @@ +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 status = (error as { status?: number }).status; + return status === 403 || status === 404; +} + +function throwFetchError(owner: string, repo: string, error: unknown): never { + throw new Error( + `Unable to list open publication PRs for ${owner}/${repo}: ${githubErrorMessage( + error, + )}`, + ); +} From ffa0ca2722b9a4cdbeb01ea0c1b32d628832c702 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Mon, 21 Sep 2026 16:54:53 +0200 Subject: [PATCH 2/2] fix: treat rate-limit 403 as failure when listing WAI PRs Do not fall back to unauthenticated listing when GitHub returns 403 with no remaining rate limit, while still retrying on 404 and other 403s. Co-authored-by: Cursor --- .../build-rule-approval-rows.test.ts | 18 ++-- .../__tests__/github-publish-prs.test.ts | 99 ++++++++++++++----- src/approval-report/github-publish-prs.ts | 9 +- 3 files changed, 92 insertions(+), 34 deletions(-) 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 ab99ca3..653a766 100644 --- a/src/approval-report/__tests__/build-rule-approval-rows.test.ts +++ b/src/approval-report/__tests__/build-rule-approval-rows.test.ts @@ -169,19 +169,19 @@ describe("buildRuleApprovalRows", () => { }); 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: async () => [ - { - number: 42, - html_url: "https://github.com/w3c/wcag-act-rules/pull/42", - ruleIds: ["674b10"], - }, - ], - }), + 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", diff --git a/src/approval-report/__tests__/github-publish-prs.test.ts b/src/approval-report/__tests__/github-publish-prs.test.ts index e373bfa..59f2e2b 100644 --- a/src/approval-report/__tests__/github-publish-prs.test.ts +++ b/src/approval-report/__tests__/github-publish-prs.test.ts @@ -12,6 +12,7 @@ describe("fetchOpenPublishPrs", () => { const list = jest.fn(); const listFiles = jest.fn(); const iterator = jest.fn(); + const originalGithubToken = process.env.GITHUB_TOKEN; beforeEach(() => { jest.clearAllMocks(); @@ -23,7 +24,11 @@ describe("fetchOpenPublishPrs", () => { }); afterAll(() => { - delete process.env.GITHUB_TOKEN; + if (originalGithubToken === undefined) { + delete process.env.GITHUB_TOKEN; + } else { + process.env.GITHUB_TOKEN = originalGithubToken; + } }); function mockGitHub( @@ -81,6 +86,21 @@ describe("fetchOpenPublishPrs", () => { 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([], {}); @@ -108,33 +128,63 @@ describe("fetchOpenPublishPrs", () => { expect(Octokit).toHaveBeenCalledWith({ auth: "test-token" }); }); - it("retries without auth when an installation token cannot access W3C", async () => { - process.env.GITHUB_TOKEN = "installation-token"; - iterator - .mockImplementationOnce(() => { - async function* denied() { - throw { status: 404, message: "Not Found" }; - yield { data: [] }; - } - return denied(); - }) - .mockImplementationOnce(() => { - async function* publicResponse() { - yield { data: [] }; - } - return publicResponse(); + 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 }); + }, + ); - await expect(fetchOpenPublishPrs("w3c", "wcag-act-rules")).resolves.toEqual( - [], - ); - expect(Octokit).toHaveBeenNthCalledWith(1, { - auth: "installation-token", + 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(); }); - expect(Octokit).toHaveBeenNthCalledWith(2, { auth: undefined }); + + 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("reports repository and rate-limit failures clearly", async () => { + it("does not retry unauthenticated when rate-limited with GITHUB_TOKEN", async () => { + process.env.GITHUB_TOKEN = "installation-token"; iterator.mockImplementation(() => { async function* responses() { throw { @@ -155,6 +205,9 @@ describe("fetchOpenPublishPrs", () => { 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 }); }); }); diff --git a/src/approval-report/github-publish-prs.ts b/src/approval-report/github-publish-prs.ts index 837126b..6a72791 100644 --- a/src/approval-report/github-publish-prs.ts +++ b/src/approval-report/github-publish-prs.ts @@ -141,8 +141,13 @@ function githubErrorMessage(error: unknown): string { function isAccessError(error: unknown): boolean { if (!error || typeof error !== "object") return false; - const status = (error as { status?: number }).status; - return status === 403 || status === 404; + 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 {