From eb0b538b27ee583891fafd48b71353d29f6247f3 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Thu, 17 Sep 2026 20:32:23 +0200 Subject: [PATCH 1/2] feat: upsert act-board issues and blocker sub-issues Generate deterministic rule issue bodies and sync them to act-board by [ruleId], including cross-repo Blocker sub-issues. Co-authored-by: Cursor --- package.json | 3 +- readme.md | 14 + .../__tests__/act-board.test.ts | 335 +++++++++++ src/approval-report/act-board.ts | 541 ++++++++++++++++++ src/cli/upsert-act-board.ts | 72 +++ 5 files changed, 964 insertions(+), 1 deletion(-) create mode 100644 src/approval-report/__tests__/act-board.test.ts create mode 100644 src/approval-report/act-board.ts create mode 100644 src/cli/upsert-act-board.ts diff --git a/package.json b/package.json index 64c7a4e..94fa62d 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "map-implementation": "ts-node src/cli/map-implementation.ts", "implementations-update": "ts-node src/cli/implementations-update.ts", "approval-report": "ts-node src/cli/approval-report.ts", + "upsert-act-board": "ts-node src/cli/upsert-act-board.ts", "test": "jest", "prepare": "husky install" }, @@ -79,4 +80,4 @@ ] }, "packageManager": "yarn@4.12.0" -} \ No newline at end of file +} diff --git a/readme.md b/readme.md index 687d27c..7570dec 100644 --- a/readme.md +++ b/readme.md @@ -102,3 +102,17 @@ The following commands are available for use in development: - `yarn lint`: Check code for lint errors using [ESLint](https://eslint.org/) - `yarn format`: Format the code using [Prettier](https://prettier.io/) +### Update act-board issues + +First generate the classifier JSON, then upsert the generated rule issues and +cross-repository blocker sub-issues. The second command requires a +`GITHUB_TOKEN` with write access to `act-rules/act-board` and issue read access +to `act-rules/act-rules.github.io`. + +```sh +yarn approval-report +GITHUB_TOKEN=... yarn upsert-act-board --input approval-report.json +``` + +The upsert command owns act-board issue titles, bodies, state, and sub-issue +relationships. It does not update GitHub Project fields. diff --git a/src/approval-report/__tests__/act-board.test.ts b/src/approval-report/__tests__/act-board.test.ts new file mode 100644 index 0000000..547efd1 --- /dev/null +++ b/src/approval-report/__tests__/act-board.test.ts @@ -0,0 +1,335 @@ +import { + actBoardIssueTitle, + ActBoardGitHubClient, + BoardIssue, + renderActBoardIssueBody, + ruleIdFromActBoardTitle, + SubIssue, + upsertActBoardIssues, +} from "../act-board"; +import { GitHubIssueRef, RuleApprovalRow } from "../types"; + +function issue(number: number, title: string, blocker = false): GitHubIssueRef { + return { + number, + title, + html_url: `https://github.com/act-rules/act-rules.github.io/issues/${number}`, + labelNames: blocker ? ["Blocker"] : [], + }; +} + +function row( + ruleId: string, + overrides: Partial = {}, +): RuleApprovalRow { + return { + ruleId, + name: `Rule ${ruleId}`, + ruleTypeSummary: "atomic", + waiApproved: true, + status: "Approved, current", + reviewPrUrl: null, + reportBucket: "approvedUpToDate", + implementations: ["axe-core"], + issues: [], + blockers: [], + changes: [], + approvalIsoDate: "2024-01-01", + lastUpdatedIsoDate: "2024-01-01", + ruleCommitCount: 0, + definitionCommitCount: 0, + lastApprovedSummary: "2024-01-01", + lastUpdatedSummary: "2024-01-01", + commitsBehindSummary: "0", + blockersCount: 0, + ...overrides, + }; +} + +function boardIssue( + rule: RuleApprovalRow, + overrides: Partial = {}, +): BoardIssue { + return { + number: Number.parseInt(rule.ruleId, 16) || 1, + title: actBoardIssueTitle(rule), + body: renderActBoardIssueBody(rule), + state: rule.status === "Deprecated" ? "closed" : "open", + nodeId: `BOARD_${rule.ruleId}`, + ...overrides, + }; +} + +function fakeClient( + overrides: { + issues?: BoardIssue[]; + subIssues?: Record; + blockerNodeIds?: Record; + } = {}, +): ActBoardGitHubClient & { + createBoardIssue: jest.Mock; + updateBoardIssue: jest.Mock; + getIssueNodeId: jest.Mock; + addSubIssue: jest.Mock; + removeSubIssue: jest.Mock; +} { + let nextIssueNumber = 100; + return { + listBoardIssues: jest.fn().mockResolvedValue(overrides.issues ?? []), + createBoardIssue: jest + .fn() + .mockImplementation( + async ( + _repository, + title: string, + body: string, + ): Promise => { + nextIssueNumber += 1; + return { + number: nextIssueNumber, + title, + body, + state: "open", + nodeId: `BOARD_NEW_${nextIssueNumber}`, + }; + }, + ), + updateBoardIssue: jest + .fn() + .mockImplementation( + async ( + _repository, + issueNumber: number, + update: Partial, + ): Promise => { + const existing = (overrides.issues ?? []).find( + (candidate) => candidate.number === issueNumber, + ) ?? { + number: issueNumber, + title: "", + body: null, + state: "open" as const, + nodeId: `BOARD_NEW_${issueNumber}`, + }; + return { ...existing, ...update }; + }, + ), + listSubIssues: jest + .fn() + .mockImplementation( + async (nodeId: string) => overrides.subIssues?.[nodeId] ?? [], + ), + getIssueNodeId: jest + .fn() + .mockImplementation( + async (_repository, number: number) => + overrides.blockerNodeIds?.[number] ?? `CG_${number}`, + ), + addSubIssue: jest.fn().mockResolvedValue(undefined), + removeSubIssue: jest.fn().mockResolvedValue(undefined), + }; +} + +describe("act-board issue rendering", () => { + it("uses the bracketed six-character rule id as its title key", () => { + expect(actBoardIssueTitle(row("674b10"))).toBe("[674b10] Rule 674b10"); + expect( + ruleIdFromActBoardTitle("[674b10] Role attribute has valid value"), + ).toBe("674b10"); + expect(ruleIdFromActBoardTitle("Rule 674b10")).toBeNull(); + expect(ruleIdFromActBoardTitle("[too-long] Rule")).toBeNull(); + }); + + it("renders all generated body sections deterministically", () => { + const blocker = issue(20, "Fix blocker", true); + const nonBlocker = issue(10, "Discuss wording"); + const body = renderActBoardIssueBody( + row("674b10", { + name: "Role attribute has valid value", + implementations: ["Zeta", "Alpha"], + issues: [blocker, nonBlocker], + blockers: [blocker], + changes: [ + { + hash: "b".repeat(40), + subject: "Definition update", + dateIso: "2024-03-01", + touchedRule: false, + touchedDefinitionKeys: ["role", "attribute"], + }, + { + hash: "a".repeat(40), + subject: "Rule and definition update", + dateIso: "2024-04-01", + touchedRule: true, + touchedDefinitionKeys: ["role"], + }, + ], + ruleCommitCount: 1, + definitionCommitCount: 1, + }), + ); + + expect( + body.startsWith("> [!NOTE]\n> This issue is generated by a bot."), + ).toBe(true); + expect(body).toContain("Do not edit this issue body"); + expect(body).toContain("### Rule changes (1)"); + expect(body).toContain("### Definition-only changes (1)"); + expect(body).toContain("Rule and definition update — definitions: role"); + expect(body).toContain("Definition update — definitions: attribute, role"); + expect(body).toContain("## Open non-blocker Community Group issues"); + expect(body).toContain("[#10: Discuss wording]"); + expect(body).toContain("## Open blocker Community Group issues"); + expect(body).toContain("[#20: Fix blocker]"); + expect(body.match(/\[#20: Fix blocker\]/g)).toHaveLength(1); + expect(body.indexOf("- Alpha")).toBeLessThan(body.indexOf("- Zeta")); + expect(body).toContain( + "https://www.w3.org/WAI/standards-guidelines/act/rules/674b10/proposed/", + ); + expect(body).toContain( + "https://www.w3.org/WAI/standards-guidelines/act/rules/674b10/", + ); + expect(body).toContain( + "https://github.com/act-rules/act-rules.github.io/blob/develop/_rules/674b10.md", + ); + }); +}); + +describe("upsertActBoardIssues", () => { + it("updates an existing issue found by rule-id prefix", async () => { + const currentRow = row("674b10", { name: "New rule name" }); + const existing = boardIssue(currentRow, { + title: "[674b10] Old rule name", + body: "old body", + }); + const client = fakeClient({ issues: [existing] }); + + const result = await upsertActBoardIssues([currentRow], client); + + expect(result.created).toBe(0); + expect(client.createBoardIssue).not.toHaveBeenCalled(); + expect(client.updateBoardIssue).toHaveBeenCalledWith( + expect.anything(), + existing.number, + expect.objectContaining({ + title: "[674b10] New rule name", + body: renderActBoardIssueBody(currentRow), + }), + ); + }); + + it("performs no writes when title, body, state, and sub-issues match", async () => { + const blocker = issue(7, "Shared blocker", true); + const existingRow = row("674b10", { + issues: [blocker], + blockers: [blocker], + blockersCount: 1, + }); + const existingIssue = boardIssue(existingRow); + const client = fakeClient({ + issues: [existingIssue], + subIssues: { + [existingIssue.nodeId]: [ + { + nodeId: "CG_7", + owner: "act-rules", + repo: "act-rules.github.io", + number: 7, + }, + ], + }, + }); + + const result = await upsertActBoardIssues([existingRow], client); + + expect(result.skipped).toBe(1); + expect(client.createBoardIssue).not.toHaveBeenCalled(); + expect(client.updateBoardIssue).not.toHaveBeenCalled(); + expect(client.getIssueNodeId).not.toHaveBeenCalled(); + expect(client.addSubIssue).not.toHaveBeenCalled(); + expect(client.removeSubIssue).not.toHaveBeenCalled(); + }); + + it("closes deprecated and snapshot-missing rule issues", async () => { + const deprecated = row("674b10", { status: "Deprecated" }); + const oldDeprecated = boardIssue(deprecated, { + body: "old body", + state: "open", + }); + const removed = boardIssue(row("2ee8b8")); + const client = fakeClient({ issues: [oldDeprecated, removed] }); + + const result = await upsertActBoardIssues([deprecated], client); + + expect(result.closed).toBe(2); + expect(client.updateBoardIssue).toHaveBeenCalledWith( + expect.anything(), + oldDeprecated.number, + expect.objectContaining({ state: "closed" }), + ); + expect(client.updateBoardIssue).toHaveBeenCalledWith( + expect.anything(), + removed.number, + { state: "closed" }, + ); + }); + + it("adds new blockers and removes dropped sub-issues", async () => { + const blocker = issue(12, "Current blocker", true); + const currentRow = row("674b10", { + issues: [blocker], + blockers: [blocker], + blockersCount: 1, + }); + const parent = boardIssue(currentRow); + const client = fakeClient({ + issues: [parent], + subIssues: { + [parent.nodeId]: [ + { + nodeId: "CG_99", + owner: "act-rules", + repo: "act-rules.github.io", + number: 99, + }, + ], + }, + blockerNodeIds: { 12: "CG_12" }, + }); + + const result = await upsertActBoardIssues([currentRow], client); + + expect(result.subIssuesRemoved).toBe(1); + expect(result.subIssuesAdded).toBe(1); + expect(client.removeSubIssue).toHaveBeenCalledWith(parent.nodeId, "CG_99"); + expect(client.addSubIssue).toHaveBeenCalledWith(parent.nodeId, "CG_12"); + }); + + it("attaches a shared blocker only to the lowest matching rule id", async () => { + const blocker = issue(42, "Shared blocker", true); + const low = row("2ee8b8", { + issues: [blocker], + blockers: [blocker], + blockersCount: 1, + }); + const high = row("674b10", { + issues: [blocker], + blockers: [blocker], + blockersCount: 1, + }); + const lowParent = boardIssue(low); + const highParent = boardIssue(high); + const client = fakeClient({ + issues: [highParent, lowParent], + blockerNodeIds: { 42: "CG_42" }, + }); + + await upsertActBoardIssues([high, low], client); + + expect(client.addSubIssue).toHaveBeenCalledTimes(1); + expect(client.addSubIssue).toHaveBeenCalledWith(lowParent.nodeId, "CG_42"); + expect(renderActBoardIssueBody(low)).toContain("[#42: Shared blocker]"); + expect(renderActBoardIssueBody(high)).toContain("[#42: Shared blocker]"); + }); +}); diff --git a/src/approval-report/act-board.ts b/src/approval-report/act-board.ts new file mode 100644 index 0000000..38d5f5a --- /dev/null +++ b/src/approval-report/act-board.ts @@ -0,0 +1,541 @@ +import { Octokit } from "@octokit/rest"; + +import { ChangeEntry, GitHubIssueRef, RuleApprovalRow } from "./types"; + +export const DEFAULT_BOARD_REPOSITORY = { + owner: "act-rules", + repo: "act-board", +}; + +export const DEFAULT_CG_REPOSITORY = { + owner: "act-rules", + repo: "act-rules.github.io", +}; + +const WAI_RULES_BASE = "https://www.w3.org/WAI/standards-guidelines/act/rules"; +const CG_RULES_BASE = + "https://github.com/act-rules/act-rules.github.io/blob/develop/_rules"; + +export type RepositoryRef = { + owner: string; + repo: string; +}; + +export type BoardIssue = { + number: number; + title: string; + body: string | null; + state: "open" | "closed"; + nodeId: string; + isPullRequest?: boolean; +}; + +export type SubIssue = { + nodeId: string; + owner: string; + repo: string; + number: number; +}; + +export type ActBoardGitHubClient = { + listBoardIssues(repository: RepositoryRef): Promise; + createBoardIssue( + repository: RepositoryRef, + title: string, + body: string, + ): Promise; + updateBoardIssue( + repository: RepositoryRef, + issueNumber: number, + update: { + title?: string; + body?: string; + state?: "open" | "closed"; + }, + ): Promise; + listSubIssues(parentNodeId: string): Promise; + getIssueNodeId( + repository: RepositoryRef, + issueNumber: number, + ): Promise; + addSubIssue(parentNodeId: string, subIssueNodeId: string): Promise; + removeSubIssue(parentNodeId: string, subIssueNodeId: string): Promise; +}; + +export type UpsertActBoardOptions = { + boardRepository?: RepositoryRef; + cgRepository?: RepositoryRef; +}; + +export type UpsertActBoardResult = { + created: number; + updated: number; + closed: number; + reopened: number; + skipped: number; + subIssuesAdded: number; + subIssuesRemoved: number; +}; + +export function actBoardIssueTitle(row: RuleApprovalRow): string { + return `[${row.ruleId}] ${row.name}`; +} + +/** Return the six-character rule id used as the act-board issue upsert key. */ +export function ruleIdFromActBoardTitle(title: string): string | null { + const match = /^\[([a-z0-9]{6})\](?:\s|$)/i.exec(title); + return match ? match[1].toLowerCase() : null; +} + +export function renderActBoardIssueBody(row: RuleApprovalRow): string { + const nonBlockers = row.issues.filter( + (issue) => !row.blockers.some((blocker) => blocker.number === issue.number), + ); + const lines = [ + "> [!NOTE]", + "> This issue is generated by a bot. Do not edit this issue body; leave comments instead.", + "", + `**Status:** ${row.status}`, + "", + "## Rule links", + "", + `- [Proposed WAI rule](${WAI_RULES_BASE}/${row.ruleId}/proposed/)`, + ]; + + if (row.waiApproved) { + lines.push(`- [Approved WAI rule](${WAI_RULES_BASE}/${row.ruleId}/)`); + } + lines.push( + `- [Community Group rule file](${CG_RULES_BASE}/${row.ruleId}.md)`, + "", + "## Complete implementations", + "", + ); + if (row.implementations.length === 0) { + lines.push("_None._"); + } else { + for (const implementation of [...row.implementations].sort(compareText)) { + lines.push(`- ${implementation}`); + } + } + + lines.push( + "", + "## Changes since approval", + "", + `### Rule changes (${row.ruleCommitCount})`, + "", + ); + const ruleChanges = sortedChanges( + row.changes.filter((change) => change.touchedRule), + ); + appendChanges(lines, ruleChanges); + + lines.push( + "", + `### Definition-only changes (${row.definitionCommitCount})`, + "", + ); + const definitionChanges = sortedChanges( + row.changes.filter( + (change) => + !change.touchedRule && change.touchedDefinitionKeys.length > 0, + ), + ); + appendChanges(lines, definitionChanges); + + lines.push("", "## Open non-blocker Community Group issues", ""); + appendIssues(lines, nonBlockers); + + lines.push("", "## Open blocker Community Group issues", ""); + appendIssues(lines, row.blockers); + + return `${lines.join("\n")}\n`; +} + +export async function upsertActBoardIssues( + rows: RuleApprovalRow[], + client: ActBoardGitHubClient, + options: UpsertActBoardOptions = {}, +): Promise { + const boardRepository = options.boardRepository ?? DEFAULT_BOARD_REPOSITORY; + const cgRepository = options.cgRepository ?? DEFAULT_CG_REPOSITORY; + const result: UpsertActBoardResult = { + created: 0, + updated: 0, + closed: 0, + reopened: 0, + skipped: 0, + subIssuesAdded: 0, + subIssuesRemoved: 0, + }; + + const listedIssues = (await client.listBoardIssues(boardRepository)).filter( + (issue) => !issue.isPullRequest, + ); + const managedIssues = new Map(); + for (const issue of [...listedIssues].sort((a, b) => a.number - b.number)) { + const ruleId = ruleIdFromActBoardTitle(issue.title); + if (ruleId && !managedIssues.has(ruleId)) { + managedIssues.set(ruleId, issue); + } + } + + const rowsByRuleId = new Map( + rows.map((row) => [row.ruleId.toLowerCase(), row]), + ); + for (const row of [...rows].sort((a, b) => + a.ruleId.localeCompare(b.ruleId), + )) { + const ruleId = row.ruleId.toLowerCase(); + const title = actBoardIssueTitle(row); + const body = renderActBoardIssueBody(row); + const desiredState = row.status === "Deprecated" ? "closed" : "open"; + let issue = managedIssues.get(ruleId); + + if (!issue) { + issue = await client.createBoardIssue(boardRepository, title, body); + result.created += 1; + if (desiredState === "closed") { + issue = await client.updateBoardIssue(boardRepository, issue.number, { + state: "closed", + }); + result.closed += 1; + } + managedIssues.set(ruleId, issue); + continue; + } + + const update: { + title?: string; + body?: string; + state?: "open" | "closed"; + } = {}; + if (issue.title !== title) update.title = title; + if ((issue.body ?? "") !== body) update.body = body; + if (issue.state !== desiredState) update.state = desiredState; + + if (Object.keys(update).length === 0) { + result.skipped += 1; + } else { + issue = await client.updateBoardIssue( + boardRepository, + issue.number, + update, + ); + managedIssues.set(ruleId, issue); + if (update.state === "closed") result.closed += 1; + else if (update.state === "open") result.reopened += 1; + else result.updated += 1; + } + } + + for (const [ruleId, issue] of managedIssues) { + if (rowsByRuleId.has(ruleId) || issue.state === "closed") continue; + const closed = await client.updateBoardIssue( + boardRepository, + issue.number, + { state: "closed" }, + ); + managedIssues.set(ruleId, closed); + result.closed += 1; + } + + const desiredParentByBlocker = desiredBlockerParents(rows); + const currentByParent = new Map(); + for (const issue of managedIssues.values()) { + currentByParent.set(issue.nodeId, await client.listSubIssues(issue.nodeId)); + } + + const desiredByParent = new Map>(); + const blockerNodeIds = new Map(); + for (const [blockerNumber, ruleId] of desiredParentByBlocker) { + const parent = managedIssues.get(ruleId); + if (!parent) continue; + let blockerNodeId = findCurrentSubIssueNodeId( + currentByParent, + cgRepository, + blockerNumber, + ); + if (!blockerNodeId) { + blockerNodeId = await client.getIssueNodeId(cgRepository, blockerNumber); + } + blockerNodeIds.set(blockerNumber, blockerNodeId); + const desired = desiredByParent.get(parent.nodeId) ?? new Set(); + desired.add(blockerNodeId); + desiredByParent.set(parent.nodeId, desired); + } + + for (const [parentNodeId, current] of currentByParent) { + const desired = desiredByParent.get(parentNodeId) ?? new Set(); + for (const child of current) { + if (desired.has(child.nodeId)) continue; + await client.removeSubIssue(parentNodeId, child.nodeId); + result.subIssuesRemoved += 1; + } + } + + for (const [parentNodeId, desired] of desiredByParent) { + const currentIds = new Set( + (currentByParent.get(parentNodeId) ?? []).map((child) => child.nodeId), + ); + for (const childNodeId of desired) { + if (currentIds.has(childNodeId)) continue; + await client.addSubIssue(parentNodeId, childNodeId); + result.subIssuesAdded += 1; + } + } + + return result; +} + +function desiredBlockerParents(rows: RuleApprovalRow[]): Map { + const matchingRuleIds = new Map(); + for (const row of rows) { + if (row.status === "Deprecated") continue; + for (const blocker of row.blockers) { + const matches = matchingRuleIds.get(blocker.number) ?? []; + matches.push(row.ruleId.toLowerCase()); + matchingRuleIds.set(blocker.number, matches); + } + } + + return new Map( + [...matchingRuleIds].map(([number, ruleIds]) => [ + number, + [...new Set(ruleIds)].sort(compareText)[0], + ]), + ); +} + +function findCurrentSubIssueNodeId( + currentByParent: Map, + repository: RepositoryRef, + issueNumber: number, +): string | undefined { + for (const children of currentByParent.values()) { + const match = children.find( + (child) => + child.owner.toLowerCase() === repository.owner.toLowerCase() && + child.repo.toLowerCase() === repository.repo.toLowerCase() && + child.number === issueNumber, + ); + if (match) return match.nodeId; + } + return undefined; +} + +function appendChanges(lines: string[], changes: ChangeEntry[]): void { + if (changes.length === 0) { + lines.push("_None._"); + return; + } + for (const change of changes) { + const shortHash = change.hash.slice(0, 7); + const commitUrl = `https://github.com/act-rules/act-rules.github.io/commit/${change.hash}`; + const definitions = + change.touchedDefinitionKeys.length > 0 + ? ` — definitions: ${[...change.touchedDefinitionKeys] + .sort(compareText) + .join(", ")}` + : ""; + lines.push( + `- [\`${shortHash}\`](${commitUrl}) ${change.subject}${definitions}`, + ); + } +} + +function appendIssues(lines: string[], issues: GitHubIssueRef[]): void { + if (issues.length === 0) { + lines.push("_None._"); + return; + } + for (const issue of [...issues].sort( + (a, b) => a.number - b.number || compareText(a.title, b.title), + )) { + lines.push(`- [#${issue.number}: ${issue.title}](${issue.html_url})`); + } +} + +function sortedChanges(changes: ChangeEntry[]): ChangeEntry[] { + return [...changes].sort( + (a, b) => + compareText(b.dateIso, a.dateIso) || + compareText(a.hash, b.hash) || + compareText(a.subject, b.subject), + ); +} + +function compareText(a: string, b: string): number { + return a.localeCompare(b); +} + +type GraphQlSubIssueResponse = { + node: { + subIssues: { + nodes: Array<{ + id: string; + number: number; + repository: { name: string; owner: { login: string } }; + }>; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + }; + } | null; +}; + +export class OctokitActBoardClient implements ActBoardGitHubClient { + public constructor(private readonly octokit: Octokit) {} + + public async listBoardIssues( + repository: RepositoryRef, + ): Promise { + const issues: BoardIssue[] = []; + const responses = this.octokit.paginate.iterator( + this.octokit.rest.issues.listForRepo, + { + ...repository, + state: "all", + per_page: 100, + }, + ); + for await (const response of responses) { + for (const issue of response.data) { + issues.push({ + number: issue.number, + title: issue.title, + body: issue.body ?? null, + state: issue.state as "open" | "closed", + nodeId: issue.node_id, + isPullRequest: Boolean(issue.pull_request), + }); + } + } + return issues; + } + + public async createBoardIssue( + repository: RepositoryRef, + title: string, + body: string, + ): Promise { + const { data } = await this.octokit.rest.issues.create({ + ...repository, + title, + body, + }); + return boardIssueFromResponse(data); + } + + public async updateBoardIssue( + repository: RepositoryRef, + issueNumber: number, + update: { + title?: string; + body?: string; + state?: "open" | "closed"; + }, + ): Promise { + const { data } = await this.octokit.rest.issues.update({ + ...repository, + issue_number: issueNumber, + ...update, + }); + return boardIssueFromResponse(data); + } + + public async listSubIssues(parentNodeId: string): Promise { + const subIssues: SubIssue[] = []; + let cursor: string | null = null; + do { + const response: GraphQlSubIssueResponse = await this.octokit.graphql( + `query ActBoardSubIssues($issueId: ID!, $cursor: String) { + node(id: $issueId) { + ... on Issue { + subIssues(first: 100, after: $cursor) { + nodes { + id + number + repository { name owner { login } } + } + pageInfo { hasNextPage endCursor } + } + } + } + }`, + { issueId: parentNodeId, cursor }, + ); + if (!response.node) break; + for (const issue of response.node.subIssues.nodes) { + subIssues.push({ + nodeId: issue.id, + owner: issue.repository.owner.login, + repo: issue.repository.name, + number: issue.number, + }); + } + cursor = response.node.subIssues.pageInfo.hasNextPage + ? response.node.subIssues.pageInfo.endCursor + : null; + } while (cursor); + return subIssues; + } + + public async getIssueNodeId( + repository: RepositoryRef, + issueNumber: number, + ): Promise { + const { data } = await this.octokit.rest.issues.get({ + ...repository, + issue_number: issueNumber, + }); + return data.node_id; + } + + public async addSubIssue( + parentNodeId: string, + subIssueNodeId: string, + ): Promise { + await this.octokit.graphql( + `mutation AddActBoardSubIssue($issueId: ID!, $subIssueId: ID!) { + addSubIssue(input: { issueId: $issueId, subIssueId: $subIssueId }) { + issue { id } + subIssue { id } + } + }`, + { issueId: parentNodeId, subIssueId: subIssueNodeId }, + ); + } + + public async removeSubIssue( + parentNodeId: string, + subIssueNodeId: string, + ): Promise { + await this.octokit.graphql( + `mutation RemoveActBoardSubIssue($issueId: ID!, $subIssueId: ID!) { + removeSubIssue(input: { issueId: $issueId, subIssueId: $subIssueId }) { + issue { id } + subIssue { id } + } + }`, + { issueId: parentNodeId, subIssueId: subIssueNodeId }, + ); + } +} + +function boardIssueFromResponse(data: { + number: number; + title: string; + body?: string | null; + state: string; + node_id: string; + pull_request?: unknown; +}): BoardIssue { + return { + number: data.number, + title: data.title, + body: data.body ?? null, + state: data.state as "open" | "closed", + nodeId: data.node_id, + isPullRequest: Boolean(data.pull_request), + }; +} diff --git a/src/cli/upsert-act-board.ts b/src/cli/upsert-act-board.ts new file mode 100644 index 0000000..acf33aa --- /dev/null +++ b/src/cli/upsert-act-board.ts @@ -0,0 +1,72 @@ +#!/usr/bin/env ts-node +import * as fs from "node:fs"; +import * as path from "node:path"; +import { Octokit } from "@octokit/rest"; +import { Command } from "commander"; + +import { + OctokitActBoardClient, + upsertActBoardIssues, +} from "../approval-report/act-board"; +import { RuleApprovalRow } from "../approval-report/types"; + +const program = new Command(); +program + .description( + "Upsert generated rule issues and blocker sub-issues in act-rules/act-board", + ) + .option( + "-i, --input ", + "Classifier RuleApprovalRow JSON file", + path.resolve(process.cwd(), "approval-report.json"), + ) + .option("--boardOwner ", "act-board repository owner", "act-rules") + .option("--boardRepo ", "act-board repository name", "act-board") + .option("--cgOwner ", "Community Group repository owner", "act-rules") + .option( + "--cgRepo ", + "Community Group repository name", + "act-rules.github.io", + ); + +program.parse(process.argv); +const options = program.opts(); +const inputPath = path.resolve(options.input); +const rows = readRows(inputPath); +const client = new OctokitActBoardClient( + new Octokit({ auth: process.env.GITHUB_TOKEN }), +); + +upsertActBoardIssues(rows, client, { + boardRepository: { + owner: options.boardOwner, + repo: options.boardRepo, + }, + cgRepository: { + owner: options.cgOwner, + repo: options.cgRepo, + }, +}) + .then((result) => { + console.log(`Processed ${rows.length} rules: ${JSON.stringify(result)}`); + }) + .catch((error) => { + console.error(error); + process.exit(1); + }); + +function readRows(filePath: string): RuleApprovalRow[] { + const parsed: unknown = JSON.parse(fs.readFileSync(filePath, "utf8")); + if ( + !Array.isArray(parsed) || + parsed.some( + (row) => + typeof row !== "object" || + row === null || + typeof (row as { ruleId?: unknown }).ruleId !== "string", + ) + ) { + throw new Error(`${filePath} is not a RuleApprovalRow JSON array`); + } + return parsed as RuleApprovalRow[]; +} From f45cd72e42a6c57ec36f598112a8c6c175c143a8 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Thu, 17 Sep 2026 22:31:36 +0200 Subject: [PATCH 2/2] fix: address act-board upsert review on #70 - Link Community Group rule files by their real `slug-ruleId.md` filename, now persisted on `RuleApprovalRow`. - Escape Markdown link delimiters and collapse whitespace in issue titles and commit subjects. - Compare issue bodies after normalizing CRLF and trailing whitespace. - Only count an issue as skipped when its sub-issues also matched. - Prefer an open board issue over a lower-numbered closed one per rule id and close the remaining duplicates. Co-authored-by: Cursor --- .../__tests__/act-board.test.ts | 93 ++++++++++++++++++- .../build-rule-approval-rows.test.ts | 14 ++- .../__tests__/generate-report.test.ts | 1 + src/approval-report/__tests__/run.test.ts | 1 + src/approval-report/act-board.ts | 74 ++++++++++++--- .../build-rule-approval-rows.ts | 1 + src/approval-report/types.ts | 2 + 7 files changed, 172 insertions(+), 14 deletions(-) diff --git a/src/approval-report/__tests__/act-board.test.ts b/src/approval-report/__tests__/act-board.test.ts index 547efd1..4ca99db 100644 --- a/src/approval-report/__tests__/act-board.test.ts +++ b/src/approval-report/__tests__/act-board.test.ts @@ -25,6 +25,7 @@ function row( return { ruleId, name: `Rule ${ruleId}`, + filename: `rule-${ruleId}.md`, ruleTypeSummary: "atomic", waiApproved: true, status: "Approved, current", @@ -69,6 +70,7 @@ function fakeClient( ): ActBoardGitHubClient & { createBoardIssue: jest.Mock; updateBoardIssue: jest.Mock; + listSubIssues: jest.Mock; getIssueNodeId: jest.Mock; addSubIssue: jest.Mock; removeSubIssue: jest.Mock; @@ -146,6 +148,7 @@ describe("act-board issue rendering", () => { const body = renderActBoardIssueBody( row("674b10", { name: "Role attribute has valid value", + filename: "role-attribute-valid-value-674b10.md", implementations: ["Zeta", "Alpha"], issues: [blocker, nonBlocker], blockers: [blocker], @@ -191,9 +194,32 @@ describe("act-board issue rendering", () => { "https://www.w3.org/WAI/standards-guidelines/act/rules/674b10/", ); expect(body).toContain( - "https://github.com/act-rules/act-rules.github.io/blob/develop/_rules/674b10.md", + "https://github.com/act-rules/act-rules.github.io/blob/develop/_rules/role-attribute-valid-value-674b10.md", ); }); + + it("escapes link-breaking characters in issue titles and commit subjects", () => { + const body = renderActBoardIssueBody( + row("674b10", { + issues: [issue(10, "Fix [role] (aria)\nsecond line")], + changes: [ + { + hash: "c".repeat(40), + subject: "fix: [role] handling (again)", + dateIso: "2024-05-01", + touchedRule: true, + touchedDefinitionKeys: [], + }, + ], + ruleCommitCount: 1, + }), + ); + + expect(body).toContain( + "- [#10: Fix \\[role\\] (aria\\) second line](https://github.com/act-rules/act-rules.github.io/issues/10)", + ); + expect(body).toContain("fix: \\[role\\] handling (again\\)"); + }); }); describe("upsertActBoardIssues", () => { @@ -251,6 +277,19 @@ describe("upsertActBoardIssues", () => { expect(client.removeSubIssue).not.toHaveBeenCalled(); }); + it("treats CRLF and trailing whitespace in the stored body as unchanged", async () => { + const existingRow = row("674b10"); + const existingIssue = boardIssue(existingRow, { + body: `${renderActBoardIssueBody(existingRow).replace(/\n/g, "\r\n")} `, + }); + const client = fakeClient({ issues: [existingIssue] }); + + const result = await upsertActBoardIssues([existingRow], client); + + expect(result.skipped).toBe(1); + expect(client.updateBoardIssue).not.toHaveBeenCalled(); + }); + it("closes deprecated and snapshot-missing rule issues", async () => { const deprecated = row("674b10", { status: "Deprecated" }); const oldDeprecated = boardIssue(deprecated, { @@ -302,10 +341,62 @@ describe("upsertActBoardIssues", () => { expect(result.subIssuesRemoved).toBe(1); expect(result.subIssuesAdded).toBe(1); + expect(result.skipped).toBe(0); expect(client.removeSubIssue).toHaveBeenCalledWith(parent.nodeId, "CG_99"); expect(client.addSubIssue).toHaveBeenCalledWith(parent.nodeId, "CG_12"); }); + it("manages the open duplicate and closes the remaining ones", async () => { + const currentRow = row("674b10", { name: "New rule name" }); + const closedLow = boardIssue(currentRow, { + number: 4, + title: "[674b10] Old rule name", + body: "old body", + state: "closed", + nodeId: "BOARD_CLOSED_LOW", + }); + const openHigh = boardIssue(currentRow, { + number: 9, + title: "[674b10] Old rule name", + body: "old body", + state: "open", + nodeId: "BOARD_OPEN_HIGH", + }); + const openExtra = boardIssue(currentRow, { + number: 12, + title: "[674b10] Another duplicate", + body: "old body", + state: "open", + nodeId: "BOARD_OPEN_EXTRA", + }); + const client = fakeClient({ issues: [openExtra, closedLow, openHigh] }); + + const result = await upsertActBoardIssues([currentRow], client); + + expect(client.updateBoardIssue).toHaveBeenCalledWith( + expect.anything(), + openHigh.number, + expect.objectContaining({ + title: "[674b10] New rule name", + body: renderActBoardIssueBody(currentRow), + }), + ); + expect(client.updateBoardIssue).toHaveBeenCalledWith( + expect.anything(), + openExtra.number, + { state: "closed" }, + ); + expect(client.updateBoardIssue).not.toHaveBeenCalledWith( + expect.anything(), + closedLow.number, + expect.anything(), + ); + expect(result.updated).toBe(1); + expect(result.closed).toBe(1); + expect(client.listSubIssues).toHaveBeenCalledWith(openHigh.nodeId); + expect(client.listSubIssues).not.toHaveBeenCalledWith(closedLow.nodeId); + }); + it("attaches a shared blocker only to the lowest matching rule id", async () => { const blocker = issue(42, "Shared blocker", true); const low = row("2ee8b8", { 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 3eab952..c536532 100644 --- a/src/approval-report/__tests__/build-rule-approval-rows.test.ts +++ b/src/approval-report/__tests__/build-rule-approval-rows.test.ts @@ -20,7 +20,7 @@ function atomicPage( return { body: "", markdownAST: emptyMdAst, - filename: `${id}.md`, + filename: `rule-${id}.md`, assets: {}, frontmatter: { id, @@ -91,6 +91,18 @@ describe("buildRuleApprovalRows", () => { }); }); + it("keeps the rule file name so links can point at the real file", async () => { + const rows = await buildRuleApprovalRows( + baseOpts, + mockDeps({ + getRulePages: () => [atomicPage("674b10")], + loadCompleteImplementationsByRuleId: () => ({ "674b10": ["axe"] }), + pathRelativeToRepo: () => "_rules/rule-674b10.md", + }), + ); + expect(rows[0].filename).toBe("rule-674b10.md"); + }); + it("strips issue body from row issues", async () => { const rows = await buildRuleApprovalRows( baseOpts, diff --git a/src/approval-report/__tests__/generate-report.test.ts b/src/approval-report/__tests__/generate-report.test.ts index f69f201..70d167b 100644 --- a/src/approval-report/__tests__/generate-report.test.ts +++ b/src/approval-report/__tests__/generate-report.test.ts @@ -10,6 +10,7 @@ function baseRow( return { ruleId, name: ruleId, + filename: `rule-${ruleId}.md`, ruleTypeSummary: "atomic", waiApproved: false, status: "Approved, current", diff --git a/src/approval-report/__tests__/run.test.ts b/src/approval-report/__tests__/run.test.ts index 4bdd65c..e3261a5 100644 --- a/src/approval-report/__tests__/run.test.ts +++ b/src/approval-report/__tests__/run.test.ts @@ -46,6 +46,7 @@ describe("runApprovalReport", () => { { ruleId: "only", name: "Only rule", + filename: "only-rule-only.md", ruleTypeSummary: "atomic", waiApproved: true, status: "Approved, current", diff --git a/src/approval-report/act-board.ts b/src/approval-report/act-board.ts index 38d5f5a..0e88864 100644 --- a/src/approval-report/act-board.ts +++ b/src/approval-report/act-board.ts @@ -106,7 +106,7 @@ export function renderActBoardIssueBody(row: RuleApprovalRow): string { lines.push(`- [Approved WAI rule](${WAI_RULES_BASE}/${row.ruleId}/)`); } lines.push( - `- [Community Group rule file](${CG_RULES_BASE}/${row.ruleId}.md)`, + `- [Community Group rule file](${CG_RULES_BASE}/${row.filename})`, "", "## Complete implementations", "", @@ -173,13 +173,8 @@ export async function upsertActBoardIssues( const listedIssues = (await client.listBoardIssues(boardRepository)).filter( (issue) => !issue.isPullRequest, ); - const managedIssues = new Map(); - for (const issue of [...listedIssues].sort((a, b) => a.number - b.number)) { - const ruleId = ruleIdFromActBoardTitle(issue.title); - if (ruleId && !managedIssues.has(ruleId)) { - managedIssues.set(ruleId, issue); - } - } + const { managedIssues, duplicateIssues } = selectManagedIssues(listedIssues); + const unwrittenIssueNodeIds = new Set(); const rowsByRuleId = new Map( rows.map((row) => [row.ruleId.toLowerCase(), row]), @@ -212,11 +207,11 @@ export async function upsertActBoardIssues( state?: "open" | "closed"; } = {}; if (issue.title !== title) update.title = title; - if ((issue.body ?? "") !== body) update.body = body; + if (normalizeBody(issue.body) !== normalizeBody(body)) update.body = body; if (issue.state !== desiredState) update.state = desiredState; if (Object.keys(update).length === 0) { - result.skipped += 1; + unwrittenIssueNodeIds.add(issue.nodeId); } else { issue = await client.updateBoardIssue( boardRepository, @@ -241,6 +236,14 @@ export async function upsertActBoardIssues( result.closed += 1; } + for (const duplicate of duplicateIssues) { + if (duplicate.state === "closed") continue; + await client.updateBoardIssue(boardRepository, duplicate.number, { + state: "closed", + }); + result.closed += 1; + } + const desiredParentByBlocker = desiredBlockerParents(rows); const currentByParent = new Map(); for (const issue of managedIssues.values()) { @@ -271,6 +274,7 @@ export async function upsertActBoardIssues( for (const child of current) { if (desired.has(child.nodeId)) continue; await client.removeSubIssue(parentNodeId, child.nodeId); + unwrittenIssueNodeIds.delete(parentNodeId); result.subIssuesRemoved += 1; } } @@ -282,13 +286,49 @@ export async function upsertActBoardIssues( for (const childNodeId of desired) { if (currentIds.has(childNodeId)) continue; await client.addSubIssue(parentNodeId, childNodeId); + unwrittenIssueNodeIds.delete(parentNodeId); result.subIssuesAdded += 1; } } + result.skipped = unwrittenIssueNodeIds.size; + return result; } +/** + * Pick the board issue to manage per rule id, preferring an open issue over a + * lower-numbered closed one. Remaining issues are duplicates to close. + */ +function selectManagedIssues(issues: BoardIssue[]): { + managedIssues: Map; + duplicateIssues: BoardIssue[]; +} { + const issuesByRuleId = new Map(); + for (const issue of [...issues].sort((a, b) => a.number - b.number)) { + const ruleId = ruleIdFromActBoardTitle(issue.title); + if (!ruleId) continue; + const matches = issuesByRuleId.get(ruleId) ?? []; + matches.push(issue); + issuesByRuleId.set(ruleId, matches); + } + + const managedIssues = new Map(); + const duplicateIssues: BoardIssue[] = []; + for (const [ruleId, matches] of issuesByRuleId) { + const managed = + matches.find((issue) => issue.state === "open") ?? matches[0]; + managedIssues.set(ruleId, managed); + duplicateIssues.push(...matches.filter((issue) => issue !== managed)); + } + return { managedIssues, duplicateIssues }; +} + +/** GitHub can return bodies with CRLF or trimmed trailing whitespace. */ +function normalizeBody(body: string | null | undefined): string { + return (body ?? "").replace(/\r\n/g, "\n").trimEnd(); +} + function desiredBlockerParents(rows: RuleApprovalRow[]): Map { const matchingRuleIds = new Map(); for (const row of rows) { @@ -340,7 +380,7 @@ function appendChanges(lines: string[], changes: ChangeEntry[]): void { .join(", ")}` : ""; lines.push( - `- [\`${shortHash}\`](${commitUrl}) ${change.subject}${definitions}`, + `- [\`${shortHash}\`](${commitUrl}) ${escMdText(change.subject)}${definitions}`, ); } } @@ -353,10 +393,20 @@ function appendIssues(lines: string[], issues: GitHubIssueRef[]): void { for (const issue of [...issues].sort( (a, b) => a.number - b.number || compareText(a.title, b.title), )) { - lines.push(`- [#${issue.number}: ${issue.title}](${issue.html_url})`); + lines.push( + `- [#${issue.number}: ${escMdText(issue.title)}](${issue.html_url})`, + ); } } +/** Collapse whitespace and escape delimiters that would break Markdown links. */ +function escMdText(text: string): string { + return text + .replace(/\s+/g, " ") + .trim() + .replace(/[[\])]/g, (character) => `\\${character}`); +} + function sortedChanges(changes: ChangeEntry[]): ChangeEntry[] { return [...changes].sort( (a, b) => diff --git a/src/approval-report/build-rule-approval-rows.ts b/src/approval-report/build-rule-approval-rows.ts index 361d74e..938046c 100644 --- a/src/approval-report/build-rule-approval-rows.ts +++ b/src/approval-report/build-rule-approval-rows.ts @@ -178,6 +178,7 @@ export async function buildRuleApprovalRows( rows.push({ ruleId, name: rule.frontmatter.name, + filename: rule.filename, ruleTypeSummary, compositeInputs: rule.frontmatter.rule_type === "composite" diff --git a/src/approval-report/types.ts b/src/approval-report/types.ts index 731fb54..6d8d5bd 100644 --- a/src/approval-report/types.ts +++ b/src/approval-report/types.ts @@ -45,6 +45,8 @@ export type RuleStatus = export type RuleApprovalRow = { ruleId: string; name: string; + /** Rule file name within `_rules`, including the `-ruleId.md` suffix. */ + filename: string; ruleTypeSummary: RuleTypeSummary; /** Set for composite rules only: atomic ids from `input_rules` (for table ordering). */ compositeInputs?: string[];