From 0ca98e2e75ff7ff945b6b933d1cbda812a612abc Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Mon, 5 Oct 2026 01:01:20 -0400 Subject: [PATCH] build(scripts): guard the generated markdown tables with a round trip instead of a regex The sections-table and policy-defaults-table regions admitted authored text their renderers never write, so a hand edit inside the markers was erased on the next regeneration instead of refused. The sections-table shape used `[^\n]*` cells and admitted a pipe, a carriage return, a sixth cell, a blank cell, a repeated section with other prose, and an Undeclared default outside the three displays. The policy-defaults-table shape admitted an override naming the same policy as the default, a keep row above a delete row, and a repeated section. Both shapes are now one shared round trip, `tableRoundTrip` in lib/markdown-table.ts: the body splits into rows and cells by the inverse of `tableRow()`, the rows pass the table rule (the header's column count, the cell rule `tableCell()` writes by, a key span opening every row, no key repeated), a per-table parser turns the cells into the record the table renders from, and the records re-rendered must reproduce the body byte for byte. The refusal names the first differing line through `renderedMismatch`, now shared with the action.yml round trip. The Defaults table re-renders through its real renderer, so the delete-first order comes from the renderer, and its parser checks the opposite-policy binding through the same `OPPOSITE` table the renderer writes from. The Sections table re-renders through the table helper with the prose cells opaque, since its PAT cell paraphrases prose no parser inverts. The helper is the grammar on both sides. `tableFault()` is the one statement of the table rule: `renderTable()` throws its text and `tableRoundTrip()` refuses by the same rule, so a renderer cannot write a body its own guard would refuse, which would otherwise turn the next regeneration of the committed page into a throw. The Defaults statement `policyTableFault()` adds each row's prose by `proseFault`, the trimmed-blank rule `tableCell()` uses, which the parser consults too, since blank prose renders `delete ()` or a bare policy span that the cell rule admits. A compile-time proof of non-blank prose was tried and dropped: a structural type cannot keep a hidden property out of a parameter, so only the run-time check closes the boundary. Two property-style tests hold both sides to the statement over generated inputs (keys inside and outside the key span, both policies, prose with blanks, padding, pipes, carriage returns, and a Unicode separator, duplicate and reordered sections): the renderer throws exactly the statement's fault, the guard admits what the renderer wrote, and the guard refuses the same cells joined without the renderer whenever the statement faults, so a check added to one side only turns a generated input red. A rule test in each registry's suite refuses a RegExp body on any region whose rendering is a table. Refusal tables for both regions pin each input above: nine were admitted by the regexes on main, the rest were refused with the generic message and now name the cell or line. --- .github/scripts/gen-action-docs.ts | 132 +++++++++++---- .github/scripts/gen-docs.ts | 47 ++++-- .github/scripts/lib/generated-regions.ts | 12 ++ .github/scripts/lib/markdown-table.ts | 135 ++++++++++++++- test/scripts/gen-action-docs.test.ts | 205 +++++++++++++++++++++-- test/scripts/gen-docs.test.ts | 157 ++++++++++++++++- 6 files changed, 619 insertions(+), 69 deletions(-) diff --git a/.github/scripts/gen-action-docs.ts b/.github/scripts/gen-action-docs.ts index 9d8d438d3..870862a0d 100644 --- a/.github/scripts/gen-action-docs.ts +++ b/.github/scripts/gen-action-docs.ts @@ -30,9 +30,14 @@ import { } from "../../src/sections/contract/permissions.js"; import { SECTIONS } from "../../src/sections/registry.js"; import { agree } from "../../src/text.js"; +import type { UndeclaredPolicy } from "../../src/types.js"; import { countWord } from "./lib/count-word.js"; -import { type GeneratedRegion, regenerateRegions } from "./lib/generated-regions.js"; -import { tableCell } from "./lib/markdown-table.js"; +import { + type GeneratedRegion, + regenerateRegions, + renderedMismatch, +} from "./lib/generated-regions.js"; +import { renderTable, tableFault, tableRoundTrip } from "./lib/markdown-table.js"; const ROOT = join(import.meta.dir, "..", ".."); @@ -174,30 +179,102 @@ export interface PolicyRowProse { readonly override: string; } -const DEFAULTS_TABLE_HEADER = "| Section | Default | The override buys you |\n|---|---|---|"; +/** + * Why a row's prose cannot be rendered: blank prose would render `delete ()` or a bare policy span, cells + * tableCell() admits since the policy word fills them. The renderer refuses it at its own boundary and the parser + * through this same check, by the trimmed-blank rule tableCell() uses, so what the renderer writes is exactly what + * the guard reads back. A structural type cannot vouch for a property a narrowing hid, so the value is inspected + * as unknown. + */ +function proseFault(row: PolicyRowProse): string | undefined { + const blank = (value: unknown): boolean => typeof value !== "string" || value.trim() === ""; + return blank(row.override) || (row.caveat !== undefined && blank(row.caveat)) + ? "has a blank caveat or override" + : undefined; +} + +const OPPOSITE: Readonly> = { + delete: "keep", + keep: "delete", +}; + +export const DEFAULTS_TABLE_HEADER = "| Section | Default | The override buys you |\n|---|---|---|"; + +/** One Defaults row: the section and the prose its cells carry. */ +export interface PolicyRow { + readonly section: KnobbedSection; + readonly prose: PolicyRowProse; +} + +/** A row's three cells, as the renderer writes them and the guard reads them back. */ +export function policyCells(row: PolicyRow): readonly string[] { + const caveat = row.prose.caveat === undefined ? "" : ` (${row.prose.caveat})`; + return [ + `\`${row.section.key}\``, + `${row.section.undeclaredDefault}${caveat}`, + `\`${OPPOSITE[row.section.undeclaredDefault]}\`: ${row.prose.override}`, + ]; +} + +/** + * The one statement of what a Defaults table may hold: each row's prose by proseFault(), then the table rule over + * the cells. The renderer consults it whole before writing; the guard consults the same two rules as it parses, + * so a test can hold both sides to it. + */ +export function policyTableFault(rows: readonly PolicyRow[]): string | undefined { + for (const row of rows) { + const fault = proseFault(row.prose); + if (fault !== undefined) { + return `the "${row.section.key}" Defaults row ${fault}`; + } + } + return tableFault(DEFAULTS_TABLE_HEADER, rows.map(policyCells)); +} export function renderPolicyDefaultsTable( sections: readonly KnobbedSection[], prose: Readonly>, ): string { - const rows = deleteFirst(sections).map((section) => { + const rows = deleteFirst(sections).map((section): PolicyRow => { const text = prose[section.key]; if (text === undefined) { throw new Error(`no Defaults-per-section prose for the "${section.key}" section`); } - const caveat = text.caveat === undefined ? "" : ` (${text.caveat})`; - const opposite = section.undeclaredDefault === "delete" ? "keep" : "delete"; - const cells = [ - `\`${section.key}\``, - tableCell(`${section.undeclaredDefault}${caveat}`, `the ${section.key} Default cell`), - tableCell( - `\`${opposite}\`: ${text.override}`, - `the ${section.key} "The override buys you" cell`, - ), - ]; - return `| ${cells.join(" | ")} |`; + // Read once, so the fault check and the cells see the same values. + return { section, prose: { caveat: text.caveat, override: text.override } }; }); - return [DEFAULTS_TABLE_HEADER, ...rows].join("\n"); + const fault = policyTableFault(rows); + if (fault !== undefined) { + throw new Error(fault); + } + return renderTable(DEFAULTS_TABLE_HEADER, rows.map(policyCells)); +} + +/** A Defaults row read back to the section and prose it renders from; the delete-first order is then the + * renderer's to check, through the byte compare. The dotAll flag keeps the two Unicode line separators + * tableCell() admits inside the prose. */ +function policyRow( + cells: readonly string[], + key: string, +): { readonly section: KnobbedSection; readonly prose: PolicyRowProse } | string { + const policy = /^(delete|keep)(?: \((.*)\))?$/s.exec(cells[1] ?? ""); + const buys = /^`(delete|keep)`: (.*)$/s.exec(cells[2] ?? ""); + if (policy === null) { + return "states no delete or keep default"; + } + if (buys === null) { + return "names no policy span before its override prose"; + } + const undeclaredDefault = policy[1] === "delete" ? "delete" : "keep"; + if (buys[1] !== OPPOSITE[undeclaredDefault]) { + return `names \`${buys[1]}\` where the override is the opposite policy, \`${OPPOSITE[undeclaredDefault]}\``; + } + const row = { caveat: policy[2], override: buys[2] ?? "" }; + const fault = proseFault(row); + if (fault !== undefined) { + return `${fault}, which the renderer refuses`; + } + return { section: { key, undeclaredDefault }, prose: row }; } /** The row every secret family shares: the value is write-only, so a wrong delete is a loss, not a drift. */ @@ -347,10 +424,6 @@ function blockShape(lines: string): RegExp { return new RegExp(String.raw`^\n(?:${lines}|\n)?$`); } -function tableShape(header: string, cells: string): RegExp { - return blockShape(String.raw`${RegExp.escape(header)}\n(?:\| ${cells} \|\n)*`); -} - /** The emitter is its own grammar: a body is admitted when re-rendering the declarations it parses to reproduces it * byte for byte, so no spelling the emitter does not write gets through. A freshly placed region holds "\n" and * renders next. */ @@ -386,15 +459,7 @@ function roundTrip( if (typeof declared === "string") { return `it is not a set of ${key} declarations (${declared})`; } - const rendered = `\n${render(declared)}\n`; - if (rendered === body) { - return undefined; - } - const authored = body.split("\n"); - const expected = rendered.split("\n"); - const differing = authored.findIndex((line, i) => line !== expected[i]); - const at = differing === -1 ? authored.length : differing; - return `line ${at} reads ${JSON.stringify(authored[at] ?? "")} where the generator writes ${JSON.stringify(expected[at] ?? "")}`; + return renderedMismatch(body, `\n${render(declared)}\n`); }; } @@ -434,10 +499,11 @@ export const GENERATED_REGIONS: Readonly + renderPolicyDefaultsTable( + rows.map((row) => row.section), + Object.fromEntries(rows.map((row) => [row.section.key, row.prose])), + ), ), render: block(() => renderPolicyDefaultsTable(knobbedSections(), POLICY_ROW_PROSE)), }, diff --git a/.github/scripts/gen-docs.ts b/.github/scripts/gen-docs.ts index 9d67bf98f..ae9dee8a9 100644 --- a/.github/scripts/gen-docs.ts +++ b/.github/scripts/gen-docs.ts @@ -16,7 +16,7 @@ import { readArchitecture, renderArchitectureMermaid } from "./arch-lint.js"; import { COVERAGE_DATA, type CoverageData } from "./coverage-data.js"; import { ENDPOINT_ANCHORS, type EndpointAnchors } from "./endpoint-docs.js"; import { type GeneratedRegion, regenerateRegions, regionBounds } from "./lib/generated-regions.js"; -import { tableCell } from "./lib/markdown-table.js"; +import { renderTable, tableCell, tableRoundTrip, tableRow } from "./lib/markdown-table.js"; const ROOT = join(import.meta.dir, "..", ".."); export const COVERAGE_PATH = "docs/reference/coverage.md"; @@ -86,27 +86,37 @@ export function renderPatCell(grant: string): string { return `${cell}; ${rendered}`; } -const TABLE_HEADER = +export const SECTIONS_TABLE_HEADER = "| Section | Endpoints | PAT permission | Undeclared default | Notes |\n|---|---|---|---|---|"; -export function renderSectionsTable( +/** + * The cells of each section's row, as the renderer writes them and the guard reads them back; the table rule + * judges them. + */ +export function sectionsCells( sections: readonly SectionsTableRow[], docs: Readonly>>, -): string { - const rows = sections.map((section) => { +): ReadonlyArray { + return sections.map((section) => { const doc = docs[section.key]; if (doc === undefined) { throw new Error(`gen-docs: section "${section.key}" has no docs entry`); } return [ `\`${section.key}\``, - tableCell(doc.sections_table.endpoints, `the ${section.key} Endpoints cell`), - tableCell(renderPatCell(sectionGrant(section)), `the ${section.key} PAT permission cell`), + doc.sections_table.endpoints, + renderPatCell(sectionGrant(section)), UNDECLARED_DEFAULT_DISPLAY[section.undeclaredDefault], - tableCell(doc.sections_table.notes, `the ${section.key} Notes cell`), + doc.sections_table.notes, ]; }); - return [TABLE_HEADER, ...rows.map((cells) => `| ${cells.join(" | ")} |`)].join("\n"); +} + +export function renderSectionsTable( + sections: readonly SectionsTableRow[], + docs: Readonly>>, +): string { + return renderTable(SECTIONS_TABLE_HEADER, sectionsCells(sections, docs)); } /** Text for inside a markdown code span: a cell, and a backtick would close the span early. */ @@ -244,7 +254,7 @@ function renderSection( `[\`${section.key}\`](${SECTIONS_PAGE})${keys}`, tableCell(renderCalls(section, row, claimed, carrier, anchors), `${where}'s Endpoints cell`), ]; - return `| ${cells.join(" | ")} |`; + return tableRow(cells); }); const declared = [...Object.keys(section.endpoints), ...Object.keys(section.graphql ?? {})]; const unclaimed = declared.filter((role) => !claimed.has(role)); @@ -310,7 +320,7 @@ export function renderCoverage( tableCell(row.endpoints.join(", "), `${where}'s Endpoints cell`), tableCell(row.why, `${where}'s Why cell`), ]; - return `| ${cells.join(" | ")} |`; + return tableRow(cells); }), ]; return [ @@ -407,14 +417,21 @@ function patFormUrl(): string { ); } -// The body shape is this generator's own output (or an empty body), built from the renderer constants, so a marker -// moved over authored prose or another table fails instead of erasing it. +// The three prose cells are opaque; a row is this table's when its Undeclared default cell is one of the displays, +// the one cell the table rule does not judge. +function sectionsRow(cells: readonly string[]): readonly string[] | string { + if (!Object.values(UNDECLARED_DEFAULT_DISPLAY).includes(cells[3] ?? "")) { + return "shows an Undeclared default the renderer has no display for"; + } + return cells; +} + function sectionsTableRegion(name: string, heading: string): GeneratedRegion { return { name, placement: { kind: "under-heading", heading }, - body: new RegExp( - String.raw`^\n(?:${RegExp.escape(TABLE_HEADER)}\n(?:\| \x60[a-z_]+\x60 \| [^\n]* \|\n)*)?$`, + body: tableRoundTrip(SECTIONS_TABLE_HEADER, sectionsRow, (rows) => + renderTable(SECTIONS_TABLE_HEADER, rows), ), render: () => `\n${renderSectionsTable(SECTIONS, DOCS)}\n`, }; diff --git a/.github/scripts/lib/generated-regions.ts b/.github/scripts/lib/generated-regions.ts index 1e1977e8b..f9227616e 100644 --- a/.github/scripts/lib/generated-regions.ts +++ b/.github/scripts/lib/generated-regions.ts @@ -159,6 +159,18 @@ export function bodyRefusal(shape: BodyShape, body: string): string | undefined return shape(body); } +/** A round-trip guard's verdict: undefined when `body` is `rendered` byte for byte, else the first differing line. */ +export function renderedMismatch(body: string, rendered: string): string | undefined { + if (rendered === body) { + return undefined; + } + const authored = body.split("\n"); + const expected = rendered.split("\n"); + const differing = authored.findIndex((line, i) => line !== expected[i]); + const at = differing === -1 ? authored.length : differing; + return `line ${at} reads ${JSON.stringify(authored[at] ?? "")} where the generator writes ${JSON.stringify(expected[at] ?? "")}`; +} + interface MarkdownScan { /** Fenced code blocks, an unclosed one running to the end of the text. */ readonly fenced: readonly MarkerSpan[]; diff --git a/.github/scripts/lib/markdown-table.ts b/.github/scripts/lib/markdown-table.ts index 821607adc..faf773d6c 100644 --- a/.github/scripts/lib/markdown-table.ts +++ b/.github/scripts/lib/markdown-table.ts @@ -1,13 +1,134 @@ +// The markdown tables the generators write: a header constant over one `| a | b |` row per record. The cell rule, +// the row joiner, and the table rule are the whole grammar on both sides: renderTable() throws exactly what +// tableRoundTrip() refuses, so a renderer cannot write a body its own guard would not read back. + +import { renderedMismatch } from "./generated-regions.js"; + +/** Why `text` cannot be a cell: a pipe or a line break would split its row, a blank would leave its column empty. */ +function cellFault(text: string): string | undefined { + return text.trim() === "" || /[|\r\n]/.test(text) + ? 'is blank or contains "|" or a line break' + : undefined; +} + /** - * A markdown table cell of authored prose, refused rather than escaped: a pipe or a line break would split its row, - * a blank one would leave its column empty, and an escape would silently change the rendered text. `where` names - * the cell as its author knows it. + * `text` as a cell of authored prose, refused rather than escaped: an escape would silently change the rendered text. + * `where` names the cell as its author knows it. */ export function tableCell(text: string, where: string): string { - if (text.trim() === "" || /[|\r\n]/.test(text)) { - throw new Error( - `${where} is blank or contains "|" or a line break, which would break its table row: "${text}"`, - ); + const fault = cellFault(text); + if (fault !== undefined) { + throw new Error(`${where} ${fault}, which would break its table row: "${text}"`); } return text; } + +/** A settings key in a code span: the cell every generated table opens its rows with, written without tableCell(). */ +const KEY_SPAN = /^`([a-z_]+)`$/; + +export function tableRow(cells: readonly string[]): string { + return `| ${cells.join(" | ")} |`; +} + +/** The cells of one tableRow() line; a line of another shape yields the wrong count or fails the byte compare. */ +function rowCells(line: string): string[] { + return line + .split("|") + .slice(1, -1) + .map((cell) => cell.slice(1, -1)); +} + +function columnsOf(header: string): number { + return rowCells(header.split("\n")[0] ?? "").length; +} + +/** + * Why `rows` cannot sit under a header of `columns` cells, as the first faulty row and its fault: a wrong cell + * count, a cell tableCell() refuses, a first cell that is no key span, or a key an earlier row already used (the + * renderers walk a registry). + */ +function rowFault( + columns: number, + rows: ReadonlyArray, +): { readonly row: number; readonly fault: string } | undefined { + const keys = new Set(); + for (const [row, cells] of rows.entries()) { + if (cells.length !== columns) { + return { row, fault: `has ${cells.length} cells where the table has ${columns}` }; + } + const faulty = cells.findIndex((cell) => cellFault(cell) !== undefined); + if (faulty !== -1) { + return { row, fault: `cell ${faulty + 1} ${cellFault(cells[faulty] ?? "")}` }; + } + const key = KEY_SPAN.exec(cells[0] ?? "")?.[1]; + if (key === undefined) { + return { row, fault: "does not open with a section key in a code span" }; + } + if (keys.has(key)) { + return { row, fault: `repeats the \`${key}\` row` }; + } + keys.add(key); + } + return undefined; +} + +/** + * The one statement of what a table under `header` may hold, naming the first faulty row: renderTable() throws it + * and tableRoundTrip() refuses by the same rule, so a test can hold both sides to it. + */ +export function tableFault( + header: string, + rows: ReadonlyArray, +): string | undefined { + const faulty = rowFault(columnsOf(header), rows); + if (faulty === undefined) { + return undefined; + } + return `row ${faulty.row + 1} of the table ${faulty.fault}: ${tableRow(rows[faulty.row] ?? [])}`; +} + +export function renderTable(header: string, rows: ReadonlyArray): string { + const fault = tableFault(header, rows); + if (fault !== undefined) { + throw new Error(fault); + } + return [header, ...rows.map(tableRow)].join("\n"); +} + +/** + * The body guard of a generated table region. The body splits into rows and cells the way renderTable() joined + * them and passes the same row rule, `parseRow` turns a row's cells into the record it renders from (a string + * is the refusal), and `renderRows` must reproduce the body byte for byte, so an authored spelling the renderer + * never writes is refused rather than erased. A freshly placed region holds "\n" and renders next. + */ +export function tableRoundTrip( + header: string, + parseRow: (cells: readonly string[], key: string) => Row | string, + renderRows: (rows: readonly Row[]) => string, +): (body: string) => string | undefined { + const headerLines = header.split("\n").length; + const columns = columnsOf(header); + return (body) => { + if (body === "\n") { + return undefined; + } + const rows: Row[] = []; + if (body.startsWith(`\n${header}\n`) && body.endsWith("\n")) { + const lines = body.split("\n").slice(headerLines + 1, -1); + const cells = lines.map(rowCells); + const line = (row: number): number => row + headerLines + 1; + const faulty = rowFault(columns, cells); + if (faulty !== undefined) { + return `line ${line(faulty.row)} ${faulty.fault}`; + } + for (const [index, rowOfCells] of cells.entries()) { + const row = parseRow(rowOfCells, KEY_SPAN.exec(rowOfCells[0] ?? "")?.[1] ?? ""); + if (typeof row === "string") { + return `line ${line(index)} ${row}`; + } + rows.push(row); + } + } + return renderedMismatch(body, `\n${renderRows(rows)}\n`); + }; +} diff --git a/test/scripts/gen-action-docs.test.ts b/test/scripts/gen-action-docs.test.ts index 99eac4ae1..65dee1372 100644 --- a/test/scripts/gen-action-docs.test.ts +++ b/test/scripts/gen-action-docs.test.ts @@ -3,7 +3,13 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { parse as parseYaml } from "yaml"; import { + DEFAULTS_TABLE_HEADER, GENERATED_REGIONS, + type KnobbedSection, + type PolicyRow, + type PolicyRowProse, + policyCells, + policyTableFault, regenerateText, renderActionInputs, renderActionOutputs, @@ -20,6 +26,7 @@ import { type RegionSpec, regionBounds, } from "../../.github/scripts/lib/generated-regions.js"; +import { tableRow } from "../../.github/scripts/lib/markdown-table.js"; import { OUTPUT_DECLS } from "../../src/action/io.js"; import { INPUT_DECLS } from "../../src/flows/inputs.js"; import type { SectionMeta } from "../../src/sections/contract/module.js"; @@ -142,7 +149,20 @@ describe("undeclared-policy renderers", () => { renderPolicyDefaultsTable(sections.slice(1, 2), { labels: { override: "manage a core set | or two" }, }), - ).toThrow('the labels "The override buys you" cell'); + ).toThrow('row 1 of the table cell 3 is blank or contains "|" or a line break'); + // The renderer refuses at its own boundary what its guard could never read back: a blank caveat or override, + // whatever the type said about the value, since a narrowing can hide a property. + const labels = sections.slice(1, 2); + const empty = 'the "labels" Defaults row has a blank caveat or override'; + expect(() => + renderPolicyDefaultsTable(labels, { labels: { caveat: "", override: "x" } }), + ).toThrow(empty); + expect(() => renderPolicyDefaultsTable(labels, { labels: { override: "" } })).toThrow(empty); + const hidden: { readonly labels: { readonly caveat: readonly []; readonly override: "x" } } = { + labels: { caveat: [], override: "x" }, + }; + const narrowed: { readonly labels: { readonly override: "x" } } = hidden; + expect(() => renderPolicyDefaultsTable(labels, narrowed)).toThrow(empty); }); }); @@ -313,23 +333,17 @@ describe("generated files", () => { { key: "rulesets", undeclaredDefault: "keep" }, ] as const; accepts("policy-count-sentence", renderPolicyCountSentence(knobbed)); + // Parentheses, backticks, colons, and the two Unicode line separators tableCell() admits must read back as the + // opaque prose they are. accepts( "policy-defaults-table", renderPolicyDefaultsTable(knobbed, { - labels: { caveat: "Probot parity", override: "manage a core set" }, - rulesets: { override: "make the file the inventory" }, + labels: { caveat: "Probot (parity): `delete`\u2028", override: "manage a core set" }, + rulesets: { override: "make the file (`settings.yml`): the inventory\u2029" }, }), ); + accepts("policy-defaults-table", renderPolicyDefaultsTable([], {})); accepts("permissions-grant-sentence", renderGrantSentence([sectionModule("teams")])); - // Rows tableCell() refuses to write, so the shape must refuse them too or regeneration erases them. - for (const row of [ - "| `labels` | delete | `keep`: manage a core set | authored |", - "| `labels` | delete (a | b) | `keep`: manage a core set |", - "| `labels` | delete (a\rb) | `keep`: manage a core set |", - ]) { - const body = `\n| Section | Default | The override buys you |\n|---|---|---|\n${row}\n`; - expect(bodyRefusal(shapeOf("policy-defaults-table"), body), row).toBeDefined(); - } const overrideGated: SectionMeta = { ...sectionModule("labels"), endpoints: { @@ -390,6 +404,173 @@ describe("generated files", () => { } }); + // Each body is table text the renderer never writes; admitted, it would be erased on the next regeneration. + const policyRow = "| `labels` | delete (Probot parity) | `keep`: manage a core set |"; + const policyTable = (rows: string): string => + `\n| Section | Default | The override buys you |\n|---|---|---|\n${rows}\n`; + test.each<[label: string, body: string, refusal: RegExp]>([ + [ + "an override naming the default policy itself", + policyTable("| `labels` | delete | `delete`: manage a core set |"), + /line 3 names `delete` where the override is the opposite policy, `keep`/, + ], + [ + "a keep row above a delete row", + policyTable( + `| \`rulesets\` | keep | \`delete\`: make the file the inventory |\n${policyRow}`, + ), + /line 3 reads "\| `rulesets` .* where the generator writes "\| `labels` /, + ], + [ + "a fourth cell", + policyTable(`${policyRow.slice(0, -2)} | authored |`), + /line 3 has 4 cells where the table has 3/, + ], + [ + "a pipe in the caveat", + policyTable(policyRow.replace("Probot parity", "a | b")), + /line 3 has 4 cells/, + ], + [ + "a carriage return in the caveat", + policyTable(policyRow.replace("Probot parity", "a\rb")), + /line 3 cell 2 is blank or contains "\|" or a line break/, + ], + [ + "a blank Default cell", + policyTable(policyRow.replace("delete (Probot parity)", "")), + /line 3 cell 2 is blank/, + ], + [ + "a Section cell outside a code span", + policyTable(policyRow.replace("`labels`", "labels")), + /line 3 does not open with a section key/, + ], + [ + "a Default outside delete and keep", + policyTable(policyRow.replace("delete (", "deleted (")), + /line 3 states no delete or keep default/, + ], + [ + "an override without its policy span", + policyTable(policyRow.replace("`keep`: ", "")), + /line 3 names no policy span/, + ], + [ + "the same section twice", + policyTable(`${policyRow}\n${policyRow}`), + /line 4 repeats the `labels` row/, + ], + [ + "a blank caveat, which the renderer refuses", + policyTable(policyRow.replace("Probot parity", "")), + /line 3 has a blank caveat or override/, + ], + [ + "a blank override, which the renderer refuses", + policyTable(policyRow.replace("manage a core set", "")), + /line 3 has a blank caveat or override/, + ], + [ + "another table's header", + "\n| Section | Default |\n|---|---|\n", + /line 1 reads "\| Section \| Default \|" where the generator writes/, + ], + ["a body without its closing newline", policyTable(policyRow).slice(0, -1), /line 3 reads/], + ])("the Defaults table guard refuses %s, which does not round-trip", (_label, body, refusal) => { + expect(bodyRefusal(shapeOf("policy-defaults-table"), body)).toMatch(refusal); + }); + + // The equivalence the round trip promises, held to the one statement both sides consult: the renderer throws + // exactly the fault policyTableFault() states for the rows, the guard admits what the renderer wrote, and the + // guard refuses the same cells joined without the renderer whenever the statement faults. A check on one side + // only turns a generated input red. + test("the Defaults renderer and guard agree with the shared statement on every generated input", () => { + const shape = shapeOf("policy-defaults-table"); + const outcomes = new Map(); + const hold = ( + input: string, + sections: readonly KnobbedSection[], + prose: Readonly>, + ): void => { + // The renderer's delete-first order, so a fault names the same row on both sides. + const rows = ["delete", "keep"].flatMap((policy) => + sections + .filter((section) => section.undeclaredDefault === policy) + .map( + (section): PolicyRow => ({ section, prose: prose[section.key] ?? { override: "" } }), + ), + ); + const fault = policyTableFault(rows); + let rendered: string | undefined; + let thrown: string | undefined; + try { + rendered = renderPolicyDefaultsTable(sections, prose); + } catch (error) { + thrown = error instanceof Error ? error.message : String(error); + } + expect(thrown, input).toBe(fault); + if (rendered !== undefined) { + expect(bodyRefusal(shape, `\n${rendered}\n`), input).toBeUndefined(); + outcomes.set(input, "written"); + } else { + const joined = [DEFAULTS_TABLE_HEADER, ...rows.map(policyCells).map(tableRow)].join("\n"); + expect(bodyRefusal(shape, `\n${joined}\n`), input).toBeDefined(); + outcomes.set(input, "faulted"); + } + }; + const policies = ["delete", "keep"] as const; + const caveats = [ + undefined, + "Probot parity", + "", + " ", + " padded ", + "(nested) `x`", + "a | b", + "a\rb", + ]; + const overrides = ["x", "", " ", " padded ", "`keep`: y", "a | b", "y\u2028z"]; + for (const key of ["labels", "Labels", "secret_scanning_custom_patterns"]) { + for (const undeclaredDefault of policies) { + for (const caveat of caveats) { + for (const override of overrides) { + const prose = caveat === undefined ? { override } : { caveat, override }; + hold( + `${key} ${undeclaredDefault} ${JSON.stringify(prose)}`, + [{ key, undeclaredDefault }], + { + [key]: prose, + }, + ); + } + } + } + } + const labels: KnobbedSection = { key: "labels", undeclaredDefault: "delete" }; + const rulesets: KnobbedSection = { key: "rulesets", undeclaredDefault: "keep" }; + const prose = { labels: { override: "x" }, rulesets: { caveat: "c", override: "y" } }; + hold("labels twice", [labels, labels], prose); + hold("keep before delete", [rulesets, labels], prose); + hold("whitespace override", [labels], { labels: { override: " " } }); + expect(outcomes.get("labels twice")).toBe("faulted"); + expect(outcomes.get("keep before delete")).toBe("written"); + expect(outcomes.get("whitespace override")).toBe("faulted"); + const written = [...outcomes.values()].filter((outcome) => outcome === "written").length; + expect(written).toBeGreaterThan(0); + expect(outcomes.size - written).toBeGreaterThan(0); + }); + + test("a region that renders a table carries a round trip, never a hand-written grammar", () => { + // A table region's guard and its renderer share one grammar only through the round trip, so a RegExp body on a + // table region is a second grammar. + for (const region of Object.values(GENERATED_REGIONS).flat()) { + if (region.render().startsWith("\n|")) { + expect(region.body, region.name).not.toBeInstanceOf(RegExp); + } + } + }); + // Each row is YAML the emitter never writes; admitted, it would be replaced on the next regeneration. The tab inside // the quoted timestamp is literal, and the two-line description folds to one line. const inputEntry = ( diff --git a/test/scripts/gen-docs.test.ts b/test/scripts/gen-docs.test.ts index 3d1985840..042cb92b2 100644 --- a/test/scripts/gen-docs.test.ts +++ b/test/scripts/gen-docs.test.ts @@ -6,6 +6,7 @@ import type { EndpointAnchors } from "../../.github/scripts/endpoint-docs.js"; import { type CoverageSection, FACT_WORD_CAP, + PAGE_REGIONS, patFormParameters, renderCoverage, renderCoverageFile, @@ -13,7 +14,12 @@ import { renderPatCell, renderPatFormUrl, renderSectionsTable, + SECTIONS_TABLE_HEADER, + type SectionsTableRow, + sectionsCells, } from "../../.github/scripts/gen-docs.js"; +import { bodyRefusal } from "../../.github/scripts/lib/generated-regions.js"; +import { tableFault, tableRow } from "../../.github/scripts/lib/markdown-table.js"; import type { SectionDocs } from "../../src/sections/contract/docs.js"; import { ROOT } from "../root.js"; @@ -65,12 +71,159 @@ describe("renderSectionsTable", () => { renderSectionsTable([row], { labels: { sections_table: { endpoints: "labels | CRUD", notes: "" } }, }), - ).toThrow('the labels Endpoints cell is blank or contains "|" or a line break'); + ).toThrow('row 1 of the table cell 2 is blank or contains "|" or a line break'); expect(() => renderSectionsTable([row], { labels: { sections_table: { endpoints: "labels CRUD", notes: "upsert\nby name" } }, }), - ).toThrow('the labels Notes cell is blank or contains "|" or a line break'); + ).toThrow('row 1 of the table cell 5 is blank or contains "|" or a line break'); + }); +}); + +describe("the Sections table guard", () => { + const shape = + PAGE_REGIONS["docs/reference/sections.md"]?.find((region) => region.name === "sections-table") + ?.body ?? (() => "no sections-table region"); + const sectionsRow = + "| `labels` | labels CRUD | Issues: write | deleted (settable) | upsert by name |"; + const sectionsTable = (rows: string): string => + `\n| Section | Endpoints | PAT permission | Undeclared default | Notes |\n|---|---|---|---|---|\n${rows}\n`; + + test("admits the renderer's output, padded and punctuated cells included", () => { + // Surrounding blanks, parentheses, backticks, and colons are all tableCell() output and read back byte for byte. + const rendered = renderSectionsTable( + [ + { key: "labels", permission: { repo: ["issues"] }, undeclaredDefault: "delete" }, + { key: "rulesets", permission: { repo: ["administration"] }, undeclaredDefault: "keep" }, + ], + { + labels: { sections_table: { endpoints: " labels (CRUD): `x` ", notes: "a: (b) `c`" } }, + rulesets: { sections_table: { endpoints: "rulesets CRUD", notes: "n" } }, + }, + ); + expect(rendered).toContain("| labels (CRUD): `x` |"); + expect(bodyRefusal(shape, `\n${rendered}\n`)).toBeUndefined(); + expect(bodyRefusal(shape, sectionsTable("").slice(0, -1))).toBeUndefined(); + expect(bodyRefusal(shape, "\n")).toBeUndefined(); + }); + + // Each body is table text the renderer never writes; admitted, it would be erased on the next regeneration. + test.each<[label: string, body: string, refusal: RegExp]>([ + [ + "a pipe in the Notes cell", + sectionsTable(sectionsRow.replace("upsert by name", "upsert | rename")), + /line 3 has 6 cells where the table has 5/, + ], + [ + "a carriage return in the Notes cell", + sectionsTable(sectionsRow.replace("upsert by name", "upsert\rby name")), + /line 3 cell 5 is blank or contains "\|" or a line break/, + ], + [ + "a sixth cell", + sectionsTable(`${sectionsRow.slice(0, -2)} | authored |`), + /line 3 has 6 cells where the table has 5/, + ], + [ + "a blank Endpoints cell", + sectionsTable(sectionsRow.replace("labels CRUD", "")), + /line 3 cell 2 is blank/, + ], + [ + "a row without its outer pipes", + sectionsTable(sectionsRow.slice(2, -2)), + /line 3 has 3 cells where the table has 5/, + ], + [ + "an Undeclared default outside the three displays", + sectionsTable(sectionsRow.replace("deleted (settable)", "deleted")), + /line 3 shows an Undeclared default the renderer has no display for/, + ], + [ + "a Section cell outside a code span", + sectionsTable(sectionsRow.replace("`labels`", "labels")), + /line 3 does not open with a section key in a code span/, + ], + [ + "a second row for the same section with other prose", + sectionsTable(`${sectionsRow}\n${sectionsRow.replace("upsert by name", "other notes")}`), + /line 4 repeats the `labels` row/, + ], + [ + "the page's column table", + "\n| Value | Meaning |\n|---|---|\n| `untouched` | never read |\n", + /line 1 reads "\| Value \| Meaning \|" where the generator writes "\| Section \| Endpoints/, + ], + ["a blank line after the last row", sectionsTable(`${sectionsRow}\n`), /line 4 has 0 cells/], + ])("refuses %s, which does not round-trip", (_label, body, refusal) => { + expect(bodyRefusal(shape, body)).toMatch(refusal); + }); + + // The equivalence the round trip promises, held to the one statement both sides consult: the renderer throws + // exactly the fault tableFault() states for the cells, the guard admits what the renderer wrote, and the guard + // refuses the same cells joined without the renderer whenever the statement faults. A check on one side only + // turns a generated input red. + test("the Sections renderer and guard agree with the shared statement on every generated input", () => { + const displays = ["delete", "keep", "untouched"] as const; + const row = (key: "labels" | "rulesets", undeclaredDefault: (typeof displays)[number]) => + ({ key, permission: { repo: ["issues"] }, undeclaredDefault }) as const; + const outcomes = new Map(); + const hold = ( + input: string, + sections: readonly SectionsTableRow[], + text: { endpoints: string; notes: string }, + ): void => { + const docs = { labels: { sections_table: text }, rulesets: { sections_table: text } }; + const cells = sectionsCells(sections, docs); + const fault = tableFault(SECTIONS_TABLE_HEADER, cells); + let rendered: string | undefined; + let thrown: string | undefined; + try { + rendered = renderSectionsTable(sections, docs); + } catch (error) { + thrown = error instanceof Error ? error.message : String(error); + } + expect(thrown, input).toBe(fault); + if (rendered !== undefined) { + expect(bodyRefusal(shape, `\n${rendered}\n`), input).toBeUndefined(); + outcomes.set(input, "written"); + } else { + const joined = [SECTIONS_TABLE_HEADER, ...cells.map(tableRow)].join("\n"); + expect(bodyRefusal(shape, `\n${joined}\n`), input).toBeDefined(); + outcomes.set(input, "faulted"); + } + }; + const texts = ["x", "", " ", "a | b", "a\rb", " padded ", "`code`: (p)", "y\u2028z"]; + for (const display of displays) { + const lists = { + one: [row("labels", display)], + two: [row("labels", display), row("rulesets", display)], + twice: [row("labels", display), row("labels", display)], + }; + for (const [name, sections] of Object.entries(lists)) { + for (const endpoints of texts) { + for (const notes of texts) { + const input = `${display} ${name} ${JSON.stringify({ endpoints, notes })}`; + hold(input, sections, { endpoints, notes }); + } + } + } + } + expect(outcomes.get('delete twice {"endpoints":"x","notes":"x"}')).toBe("faulted"); + expect(outcomes.get('untouched one {"endpoints":"x","notes":" padded "}')).toBe("written"); + const written = [...outcomes.values()].filter((outcome) => outcome === "written").length; + expect(written).toBeGreaterThan(0); + expect(outcomes.size - written).toBeGreaterThan(0); + }); + + test("a region that renders a table carries a round trip, never a hand-written grammar", () => { + // A table region's guard and its renderer share one grammar only through the round trip, so a RegExp body on a + // table region is a second grammar. + for (const region of Object.values(PAGE_REGIONS).flat()) { + if (region.render().startsWith("\n|")) { + expect(region.body, region.name).not.toBeInstanceOf(RegExp); + } + } }); });