diff --git a/.github/workflows/qa.yaml b/.github/workflows/qa.yaml index 7777ae1..a7ed4c0 100644 --- a/.github/workflows/qa.yaml +++ b/.github/workflows/qa.yaml @@ -165,7 +165,7 @@ jobs: echo "Drift report already present in summary (from check step); skipping duplicate." else { - echo "No structured drift file found. This is a REPORT, not a gate." + echo "No structured drift report was produced. This is a REPORT, not a gate." echo "Run \`bun run explain:catalog\` and review \`git diff src/explain/catalog.ts\`." } >> "$GITHUB_STEP_SUMMARY" fi diff --git a/docs/errors/validation/unknown-attribute.md b/docs/errors/validation/unknown-attribute.md index e2bd8a1..4f848df 100644 --- a/docs/errors/validation/unknown-attribute.md +++ b/docs/errors/validation/unknown-attribute.md @@ -1,23 +1,28 @@ # `validation/unknown-attribute` -An attribute is not valid for the path/verb per /console/inspect. The -validator rejects the command before RouterOS executes it. +An attribute or argument is not valid for the requested RouterOS operation. +The validator rejects the request before RouterOS executes it. ## Context keys -Context keys vary by validation mechanism — callers must not assume every -key below is present on every `validation/unknown-attribute` rejection. -The inspect-gated paths (`/console/inspect` + `:parse`) emit the full -structured shape; the console `:parse`-only path emits a lighter shape. +Context keys vary by validation mechanism. Callers must not assume every key +below is present on every `validation/unknown-attribute` rejection. | Key | Type | Notes | | --- | ---- | ----- | -| `parameter` | `string` | Offending attribute name — `missing[0]` (first missing). Always present on the inspect-gated `validation/unknown-attribute` path; console `:parse` siblings use `command`/`detail`/`position` instead. | -| `requestedAttributes` | `string[]` | Full requested attribute/argument list supplied by the caller. Inspect-gated only. | -| `availableAttributes` | `string[]` | Attributes the path/verb exposes per `/console/inspect`. Inspect-gated only. | -| `path` | `string` | Slash-prefixed RouterOS path (e.g. `/ip/address`). Inspect-gated only. | -| `verb` | `string` | Verb when applicable (`print`, `add`, `set`, …). Absent on retrieve (path-only). Inspect-gated only. | -| `validationSource` | `string` | Inspect provenance (e.g. `/console/inspect request=child+completion`, `:put [:parse] + /console/inspect`). Inspect-gated only. | +| `parameter` | `string` | First offending name. Present on retrieve and inspect-gated rejections, and on console `:parse` when RouterOS reports `bad parameter `. | +| `requestedAttributes` | `string[]` | Full requested name list. Retrieve and inspect-gated API/execute validation only. | +| `availableAttributes` | `string[]` | Names exposed for the path/verb. Retrieve and inspect-gated API/execute validation only. | +| `path` | `string` | Slash-prefixed RouterOS path (for example `/ip/address`). Retrieve and inspect-gated API/execute validation only. | +| `verb` | `string` | Verb when applicable (`print`, `add`, `set`, …). Inspect-gated API/execute validation only; retrieve is path-only. | +| `validationSource` | `string` | Validation provenance. Inspect-gated API/execute and console `:parse` only; retrieve does not currently emit it. | +| `command` | `string` | Original CLI command. Console `:parse` only. | +| `detail` | `string` | Raw RouterOS parse diagnostic. Console `:parse` only. | + +Console `:parse` may also put RouterOS' byte location in the top-level +`error.position` field. On older RouterOS versions, an unknown name can produce +only a generic syntax diagnostic; that is reported as `validation/syntax` +because no parameter identity is available. Provenance: @@ -29,7 +34,9 @@ Provenance: ## Fix -Check the attribute name against `/console/inspect`, or use -`--list-attributes` to inspect the available properties first. Use -`--validate=false` only when intentionally probing an undocumented -RouterOS edge. +For `retrieve` and inspect-gated API/execute validation, check the name against +`/console/inspect`; `retrieve --list-attributes` lists the available properties. +For console `:parse`, compare `context.parameter` with the RouterOS parameters +for `context.command`; `context.detail` preserves the device's raw diagnostic. +Use `--validate=false` only when intentionally probing an undocumented RouterOS +edge. diff --git a/scripts/explain-catalog-drift.ts b/scripts/explain-catalog-drift.ts new file mode 100644 index 0000000..4f0d24e --- /dev/null +++ b/scripts/explain-catalog-drift.ts @@ -0,0 +1,276 @@ +/** + * Parse and report generated explain-catalog drift without fetching either + * source. Keeping this separate from the generator makes the review surface + * directly testable, including the GitHub Actions summary path. + */ + +import { appendFileSync } from "node:fs"; +import type { CatalogRow } from "./explain-catalog-data.ts"; + +export const DRIFT_REPORT_SENTINEL = ""; + +export function parseCommittedCatalog( + text: string, +): Map | null { + const match = text.match(/const ROWS = `\r?\n([\s\S]*?)\r?\n`;\r?\n/); + if (match === null) return null; + const body = match[1] ?? ""; + const out = new Map(); + for (const raw of body.split(/\r?\n/)) { + const line = raw.replace(/\r$/, ""); + if (line === "") continue; + const parts = line.split("|"); + if (parts.length > 6) return null; + const path = parts[0]; + const kind = parts[1]; + const provenance = parts[2]; + if ( + !path || + kind === undefined || + provenance === undefined || + !["menu", "command", "settings"].includes(kind) || + !["inspect", "published", "both"].includes(provenance) + ) + return null; + const row: CatalogRow = { + kind: kind as CatalogRow["kind"], + path, + provenance: provenance as CatalogRow["provenance"], + }; + const pkg = parts[3]; + const cond = parts[4]; + const syscap = parts[5]; + if (pkg) row.package = pkg; + if (cond) row.conditions = cond; + if (syscap) row.syscap = syscap; + if (out.has(path)) return null; + out.set(path, row); + } + return out; +} + +function formatGate(row: CatalogRow): string { + const parts: string[] = []; + if (row.package) parts.push(`package=${JSON.stringify(row.package)}`); + if (row.conditions) + parts.push(`conditions=${JSON.stringify(row.conditions)}`); + if (row.syscap) parts.push(`syscap=${JSON.stringify(row.syscap)}`); + return parts.length > 0 ? parts.join(" ") : "(none)"; +} + +export function buildDriftReport( + committed: Map | null, + freshRows: readonly CatalogRow[], +): string { + const fresh = new Map(freshRows.map((row) => [row.path, row])); + const lines: string[] = []; + + lines.push(DRIFT_REPORT_SENTINEL); + lines.push("### `src/explain/catalog.ts` is out of date"); + lines.push(""); + lines.push("MikroTik's CLI Reference is current-only, so this fires on any"); + lines.push("structural change upstream. It is a REPORT, not a gate: nothing"); + lines.push("is blocked, including releases."); + lines.push(""); + lines.push("To adopt the change: `bun run explain:catalog` and commit."); + lines.push("Read the diff first — a path changing kind, or losing device"); + lines.push("provenance, changes what a caller may conclude from it."); + lines.push(""); + + if (committed === null) { + lines.push( + `Fresh catalog has ${fresh.size} paths; committed file could not be parsed (binary diff).`, + ); + lines.push(""); + lines.push( + "Run `bun run explain:catalog` and review `git diff src/explain/catalog.ts`.", + ); + return lines.join("\n"); + } + + const countBy = ( + map: ReadonlyMap, + predicate: (row: CatalogRow) => boolean, + ): number => { + let n = 0; + for (const row of map.values()) if (predicate(row)) n++; + return n; + }; + + lines.push( + `Row counts — committed ${committed.size.toLocaleString("en-US")} → fresh ${fresh.size.toLocaleString("en-US")} (Δ ${(fresh.size - committed.size).toLocaleString("en-US")})`, + ); + lines.push(""); + lines.push("| Provenance | Committed | Fresh |"); + lines.push("| --- | --- | --- |"); + for (const prov of ["both", "inspect", "published"] as const) { + lines.push( + `| \`${prov}\` | ${countBy(committed, (r) => r.provenance === prov).toLocaleString("en-US")} | ${countBy(fresh, (r) => r.provenance === prov).toLocaleString("en-US")} |`, + ); + } + lines.push(""); + lines.push("| Kind | Committed | Fresh |"); + lines.push("| --- | --- | --- |"); + for (const kind of ["menu", "command", "settings"] as const) { + lines.push( + `| \`${kind}\` | ${countBy(committed, (r) => r.kind === kind).toLocaleString("en-US")} | ${countBy(fresh, (r) => r.kind === kind).toLocaleString("en-US")} |`, + ); + } + lines.push(""); + + const added: CatalogRow[] = []; + const removed: CatalogRow[] = []; + for (const [path, row] of fresh) if (!committed.has(path)) added.push(row); + for (const [path, row] of committed) if (!fresh.has(path)) removed.push(row); + added.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); + removed.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); + + const isPublishedRelated = (row: CatalogRow): boolean => + row.provenance !== "inspect"; + const addedPublished = added.filter(isPublishedRelated); + const removedPublished = removed.filter(isPublishedRelated); + + const cap = 40; + const formatRows = (rows: readonly CatalogRow[]): string[] => + rows + .slice(0, cap) + .map( + (r) => + `- \`${r.path}\` — \`${r.kind}\` \`${r.provenance}\`${r.package || r.conditions || r.syscap ? ` — ${formatGate(r)}` : ""}`, + ); + + lines.push( + `#### Published paths added (${addedPublished.length}) / removed (${removedPublished.length})`, + ); + if (addedPublished.length === 0 && removedPublished.length === 0) { + lines.push("No published-related paths added or removed."); + } else { + if (addedPublished.length > 0) { + lines.push(`Added (${addedPublished.length}):`); + lines.push(...formatRows(addedPublished)); + if (addedPublished.length > cap) + lines.push(`- … and ${addedPublished.length - cap} more`); + } + if (removedPublished.length > 0) { + lines.push(`Removed (${removedPublished.length}):`); + lines.push(...formatRows(removedPublished)); + if (removedPublished.length > cap) + lines.push(`- … and ${removedPublished.length - cap} more`); + } + if ( + added.length !== addedPublished.length || + removed.length !== removedPublished.length + ) { + lines.push(""); + lines.push( + `(Also ${added.length - addedPublished.length} inspect-only added, ${removed.length - removedPublished.length} inspect-only removed — omitted above.)`, + ); + } + } + lines.push(""); + + const provenanceFlips: { + path: string; + from: CatalogRow["provenance"]; + to: CatalogRow["provenance"]; + kind: CatalogRow["kind"]; + }[] = []; + const gateChanges: string[] = []; + const kindChanges: string[] = []; + const formatFlip = (flip: { + path: string; + from: CatalogRow["provenance"]; + to: CatalogRow["provenance"]; + kind: CatalogRow["kind"]; + }): string => + `- \`${flip.path}\` — \`${flip.from}\` → \`${flip.to}\` (\`${flip.kind}\`)`; + for (const [path, freshRow] of fresh) { + const old = committed.get(path); + if (old === undefined) continue; + if (old.provenance !== freshRow.provenance) + provenanceFlips.push({ + path, + from: old.provenance, + to: freshRow.provenance, + kind: freshRow.kind, + }); + if (old.kind !== freshRow.kind) + kindChanges.push( + `- \`${path}\` — \`${old.kind}\` → \`${freshRow.kind}\``, + ); + const gateKeys: (keyof CatalogRow)[] = ["package", "conditions", "syscap"]; + const gateDiff = gateKeys.some( + (key) => (old[key] ?? "") !== (freshRow[key] ?? ""), + ); + if (gateDiff) + gateChanges.push( + `- \`${path}\` — ${formatGate(old)} → ${formatGate(freshRow)}`, + ); + } + provenanceFlips.sort((a, b) => + a.path < b.path ? -1 : a.path > b.path ? 1 : 0, + ); + kindChanges.sort(); + gateChanges.sort(); + + lines.push(`#### Provenance changes (${provenanceFlips.length})`); + if (provenanceFlips.length === 0) lines.push("No provenance flips."); + else { + const bothToPublished = provenanceFlips.filter( + (flip) => flip.from === "both" && flip.to === "published", + ); + if (bothToPublished.length > 0) { + lines.push( + `Both → published (losing device confirmation, ${bothToPublished.length}):`, + ); + lines.push(...bothToPublished.slice(0, cap).map(formatFlip)); + if (bothToPublished.length > cap) + lines.push(`- … and ${bothToPublished.length - cap} more`); + } + const rest = provenanceFlips.filter( + (flip) => !(flip.from === "both" && flip.to === "published"), + ); + if (rest.length > 0) { + if (bothToPublished.length > 0) lines.push("Other provenance flips:"); + lines.push(...rest.slice(0, cap).map(formatFlip)); + if (rest.length > cap) lines.push(`- … and ${rest.length - cap} more`); + } + } + lines.push(""); + + if (kindChanges.length > 0) { + lines.push(`#### Kind changes (${kindChanges.length})`); + lines.push( + "These are committed → fresh changes. Cross-source kind contradictions abort before this report is built.", + ); + lines.push(...kindChanges.slice(0, cap)); + if (kindChanges.length > cap) + lines.push(`- … and ${kindChanges.length - cap} more`); + lines.push(""); + } + + lines.push(`#### Gate-string changes (${gateChanges.length})`); + if (gateChanges.length === 0) lines.push("No gate changes."); + else { + lines.push(...gateChanges.slice(0, cap)); + if (gateChanges.length > cap) + lines.push(`- … and ${gateChanges.length - cap} more`); + } + lines.push(""); + + return lines.join("\n"); +} + +/** Append without truncating earlier steps; summary publication is best-effort. */ +export function appendDriftReportToSummary( + report: string, + summaryPath: string | undefined, +): boolean { + if (!summaryPath) return false; + try { + appendFileSync(summaryPath, `${report}\n`); + return true; + } catch { + return false; + } +} diff --git a/scripts/gen-explain-catalog.ts b/scripts/gen-explain-catalog.ts index 6e46fcb..bb5f714 100644 --- a/scripts/gen-explain-catalog.ts +++ b/scripts/gen-explain-catalog.ts @@ -21,16 +21,10 @@ * small page fetches plus 15 MB of trees, about 15 seconds. */ -import { - appendFileSync, - mkdirSync, - readFileSync, - writeFileSync, -} from "node:fs"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { isAbsolute, join, relative, resolve } from "node:path"; import { build, - type CatalogRow, CLI_PREFIX, cliRefSlugs, countRowMarkers, @@ -40,6 +34,11 @@ import { render, SITEMAP_URL, } from "./explain-catalog-data.ts"; +import { + appendDriftReportToSummary, + buildDriftReport, + parseCommittedCatalog, +} from "./explain-catalog-drift.ts"; import { fetchTextWithRetry } from "./fetch-retry.ts"; import { fetchPinnedTrees, mergeTreeTypes } from "./restraml-trees.ts"; @@ -167,253 +166,6 @@ console.log( `${rows.filter((r) => r.provenance === "published").length} published)`, ); -function parseCommittedCatalog(text: string): Map | null { - const match = text.match(/const ROWS = `\r?\n([\s\S]*?)\r?\n`;\r?\n/); - if (match === null) return null; - const body = match[1] ?? ""; - const out = new Map(); - for (const raw of body.split(/\r?\n/)) { - const line = raw.replace(/\r$/, ""); - if (line === "") continue; - const parts = line.split("|"); - const path = parts[0]; - const kind = parts[1]; - const provenance = parts[2]; - if ( - path === undefined || - kind === undefined || - provenance === undefined || - !["menu", "command", "settings"].includes(kind) || - !["inspect", "published", "both"].includes(provenance) - ) - continue; - const row: CatalogRow = { - kind: kind as CatalogRow["kind"], - path, - provenance: provenance as CatalogRow["provenance"], - }; - const pkg = parts[3]; - const cond = parts[4]; - const syscap = parts[5]; - if (pkg) row.package = pkg; - if (cond) row.conditions = cond; - if (syscap) row.syscap = syscap; - out.set(path, row); - } - return out; -} - -function formatGate(row: CatalogRow): string { - const parts: string[] = []; - if (row.package) parts.push(`package=${JSON.stringify(row.package)}`); - if (row.conditions) - parts.push(`conditions=${JSON.stringify(row.conditions)}`); - if (row.syscap) parts.push(`syscap=${JSON.stringify(row.syscap)}`); - return parts.length > 0 ? parts.join(" ") : "(none)"; -} - -function buildDriftReport( - committed: Map | null, - freshRows: readonly CatalogRow[], -): string { - const fresh = new Map(freshRows.map((row) => [row.path, row])); - const lines: string[] = []; - - lines.push(""); - lines.push("### `src/explain/catalog.ts` is out of date"); - lines.push(""); - lines.push("MikroTik's CLI Reference is current-only, so this fires on any"); - lines.push("structural change upstream. It is a REPORT, not a gate: nothing"); - lines.push("is blocked, including releases."); - lines.push(""); - lines.push("To adopt the change: `bun run explain:catalog` and commit."); - lines.push("Read the diff first — a path changing kind, or losing device"); - lines.push("provenance, changes what a caller may conclude from it."); - lines.push(""); - - if (committed === null) { - lines.push( - `Fresh catalog has ${fresh.size} paths; committed file could not be parsed (binary diff).`, - ); - lines.push(""); - lines.push( - "Run `bun run explain:catalog` and review `git diff src/explain/catalog.ts`.", - ); - return lines.join("\n"); - } - - const countBy = ( - map: ReadonlyMap, - predicate: (row: CatalogRow) => boolean, - ): number => { - let n = 0; - for (const row of map.values()) if (predicate(row)) n++; - return n; - }; - - lines.push( - `Row counts — committed ${committed.size.toLocaleString("en-US")} → fresh ${fresh.size.toLocaleString("en-US")} (Δ ${(fresh.size - committed.size).toLocaleString("en-US")})`, - ); - lines.push(""); - lines.push("| Provenance | Committed | Fresh |"); - lines.push("| --- | --- | --- |"); - for (const prov of ["both", "inspect", "published"] as const) { - lines.push( - `| \`${prov}\` | ${countBy(committed, (r) => r.provenance === prov).toLocaleString("en-US")} | ${countBy(fresh, (r) => r.provenance === prov).toLocaleString("en-US")} |`, - ); - } - lines.push(""); - lines.push("| Kind | Committed | Fresh |"); - lines.push("| --- | --- | --- |"); - for (const kind of ["menu", "command", "settings"] as const) { - lines.push( - `| \`${kind}\` | ${countBy(committed, (r) => r.kind === kind).toLocaleString("en-US")} | ${countBy(fresh, (r) => r.kind === kind).toLocaleString("en-US")} |`, - ); - } - lines.push(""); - - const added: CatalogRow[] = []; - const removed: CatalogRow[] = []; - for (const [path, row] of fresh) if (!committed.has(path)) added.push(row); - for (const [path, row] of committed) if (!fresh.has(path)) removed.push(row); - added.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); - removed.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); - - const isPublishedRelated = (row: CatalogRow): boolean => - row.provenance !== "inspect"; - const addedPublished = added.filter(isPublishedRelated); - const removedPublished = removed.filter(isPublishedRelated); - - const cap = 40; - const formatRows = (rows: readonly CatalogRow[]): string[] => - rows - .slice(0, cap) - .map( - (r) => - `- \`${r.path}\` — \`${r.kind}\` \`${r.provenance}\`${r.package || r.conditions || r.syscap ? ` — ${formatGate(r)}` : ""}`, - ); - - lines.push( - `#### Published paths added (${addedPublished.length}) / removed (${removedPublished.length})`, - ); - if (addedPublished.length === 0 && removedPublished.length === 0) { - lines.push("No published-related paths added or removed."); - } else { - if (addedPublished.length > 0) { - lines.push(`Added (${addedPublished.length}):`); - lines.push(...formatRows(addedPublished)); - if (addedPublished.length > cap) - lines.push(`- … and ${addedPublished.length - cap} more`); - } - if (removedPublished.length > 0) { - lines.push(`Removed (${removedPublished.length}):`); - lines.push(...formatRows(removedPublished)); - if (removedPublished.length > cap) - lines.push(`- … and ${removedPublished.length - cap} more`); - } - if ( - added.length !== addedPublished.length || - removed.length !== removedPublished.length - ) { - lines.push(""); - lines.push( - `(Also ${added.length - addedPublished.length} inspect-only added, ${removed.length - removedPublished.length} inspect-only removed — omitted above.)`, - ); - } - } - lines.push(""); - - const provenanceFlips: { - path: string; - from: CatalogRow["provenance"]; - to: CatalogRow["provenance"]; - kind: CatalogRow["kind"]; - }[] = []; - const gateChanges: string[] = []; - const kindChanges: string[] = []; - const formatFlip = (flip: { - path: string; - from: CatalogRow["provenance"]; - to: CatalogRow["provenance"]; - kind: CatalogRow["kind"]; - }): string => - `- \`${flip.path}\` — \`${flip.from}\` → \`${flip.to}\` (\`${flip.kind}\`)`; - for (const [path, freshRow] of fresh) { - const old = committed.get(path); - if (old === undefined) continue; - if (old.provenance !== freshRow.provenance) - provenanceFlips.push({ - path, - from: old.provenance, - to: freshRow.provenance, - kind: freshRow.kind, - }); - if (old.kind !== freshRow.kind) - kindChanges.push( - `- \`${path}\` — \`${old.kind}\` → \`${freshRow.kind}\``, - ); - const gateKeys: (keyof CatalogRow)[] = ["package", "conditions", "syscap"]; - const gateDiff = gateKeys.some( - (key) => (old[key] ?? "") !== (freshRow[key] ?? ""), - ); - if (gateDiff) - gateChanges.push( - `- \`${path}\` — ${formatGate(old)} → ${formatGate(freshRow)}`, - ); - } - provenanceFlips.sort((a, b) => - a.path < b.path ? -1 : a.path > b.path ? 1 : 0, - ); - kindChanges.sort(); - gateChanges.sort(); - - lines.push(`#### Provenance changes (${provenanceFlips.length})`); - if (provenanceFlips.length === 0) lines.push("No provenance flips."); - else { - const bothToPublished = provenanceFlips.filter( - (flip) => flip.from === "both" && flip.to === "published", - ); - if (bothToPublished.length > 0) { - lines.push( - `Both → published (losing device confirmation, ${bothToPublished.length}):`, - ); - lines.push(...bothToPublished.slice(0, cap).map(formatFlip)); - if (bothToPublished.length > cap) - lines.push(`- … and ${bothToPublished.length - cap} more`); - } - const rest = provenanceFlips.filter( - (flip) => !(flip.from === "both" && flip.to === "published"), - ); - if (rest.length > 0) { - if (bothToPublished.length > 0) lines.push("Other provenance flips:"); - lines.push(...rest.slice(0, cap).map(formatFlip)); - if (rest.length > cap) lines.push(`- … and ${rest.length - cap} more`); - } - } - lines.push(""); - - if (kindChanges.length > 0) { - lines.push( - `#### Kind changes (${kindChanges.length}) — generation would normally abort on contradictions`, - ); - lines.push(...kindChanges.slice(0, cap)); - if (kindChanges.length > cap) - lines.push(`- … and ${kindChanges.length - cap} more`); - lines.push(""); - } - - lines.push(`#### Gate-string changes (${gateChanges.length})`); - if (gateChanges.length === 0) lines.push("No gate changes."); - else { - lines.push(...gateChanges.slice(0, cap)); - if (gateChanges.length > cap) - lines.push(`- … and ${gateChanges.length - cap} more`); - } - lines.push(""); - - return lines.join("\n"); -} - if (process.argv.includes("--check")) { const existing = await Bun.file(OUTPUT_PATH) .text() @@ -422,14 +174,7 @@ if (process.argv.includes("--check")) { const committed = parseCommittedCatalog(existing); const report = buildDriftReport(committed, rows); console.error(report); - const summaryPath = process.env["GITHUB_STEP_SUMMARY"]; - if (summaryPath) { - try { - appendFileSync(summaryPath, `${report}\n`); - } catch { - // summary is best-effort; the stderr report is the source of truth - } - } + appendDriftReportToSummary(report, process.env["GITHUB_STEP_SUMMARY"]); process.exit(1); } console.log("src/explain/catalog.ts matches its two sources."); diff --git a/test/unit/explain-catalog-drift.test.ts b/test/unit/explain-catalog-drift.test.ts new file mode 100644 index 0000000..e690317 --- /dev/null +++ b/test/unit/explain-catalog-drift.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { CatalogRow } from "../../scripts/explain-catalog-data.ts"; +import { + appendDriftReportToSummary, + buildDriftReport, + DRIFT_REPORT_SENTINEL, + parseCommittedCatalog, +} from "../../scripts/explain-catalog-drift.ts"; + +const row = ( + path: string, + kind: CatalogRow["kind"], + provenance: CatalogRow["provenance"], + gates: Partial> = {}, +): CatalogRow => ({ path, kind, provenance, ...gates }); + +describe("explain catalog drift report", () => { + test("parses committed generated rows identically with LF and CRLF", () => { + const lf = [ + "export const header = true;", + "const ROWS = `", + "/a|menu|both|wireless|has-cap|wifi", + "/b|command|published|||", + "`;", + "export const footer = true;", + "", + ].join("\n"); + + const expected: [string, CatalogRow][] = [ + [ + "/a", + row("/a", "menu", "both", { + package: "wireless", + conditions: "has-cap", + syscap: "wifi", + }), + ], + ["/b", row("/b", "command", "published")], + ]; + + expect([...(parseCommittedCatalog(lf)?.entries() ?? [])]).toEqual(expected); + expect([ + ...(parseCommittedCatalog(lf.replaceAll("\n", "\r\n"))?.entries() ?? []), + ]).toEqual(expected); + }); + + test("itemizes structural changes and keeps Markdown tables separate", () => { + const committed = new Map([ + ["/gone", row("/gone", "menu", "published")], + ["/inspect-gone", row("/inspect-gone", "menu", "inspect")], + ["/flip", row("/flip", "menu", "both")], + ["/gate", row("/gate", "menu", "published", { package: "old-package" })], + ["/kind", row("/kind", "menu", "both")], + ["/steady", row("/steady", "settings", "inspect")], + ]); + const fresh = [ + row("/added", "command", "published", { syscap: "wifi" }), + row("/inspect-added", "menu", "inspect"), + row("/flip", "menu", "published"), + row("/gate", "menu", "published", { package: "new-package" }), + row("/kind", "command", "both"), + row("/steady", "settings", "inspect"), + ]; + + const report = buildDriftReport(committed, fresh); + + expect(report.startsWith(`${DRIFT_REPORT_SENTINEL}\n`)).toBe(true); + expect(report).toContain("Row counts — committed 6 → fresh 6 (Δ 0)"); + expect(report).toContain("| `published` | 2 | 3 |"); + expect(report).toContain("| `command` | 0 | 2 |"); + expect(report).toContain( + "| `published` | 2 | 3 |\n\n| Kind | Committed | Fresh |", + ); + + expect(report).toContain("Published paths added (1) / removed (1)"); + expect(report).toContain( + '`/added` — `command` `published` — syscap="wifi"', + ); + expect(report).toContain("`/gone` — `menu` `published`"); + expect(report).toContain( + "(Also 1 inspect-only added, 1 inspect-only removed — omitted above.)", + ); + + expect(report).toContain( + "Both → published (losing device confirmation, 1):", + ); + expect(report).toContain("`/flip` — `both` → `published` (`menu`)"); + expect(report.match(/`\/flip` — `both` → `published`/g)?.length).toBe(1); + + expect(report).toContain("#### Kind changes (1)"); + expect(report).toContain( + "These are committed → fresh changes. Cross-source kind contradictions abort before this report is built.", + ); + expect(report).not.toContain("would normally abort on contradictions"); + expect(report).toContain("`/kind` — `menu` → `command`"); + + expect(report).toContain("#### Gate-string changes (1)"); + expect(report).toContain( + '`/gate` — package="old-package" → package="new-package"', + ); + }); + + test("keeps the sentinel and binary-diff guidance when parsing fails", () => { + expect(parseCommittedCatalog("not a generated catalog")).toBeNull(); + expect( + parseCommittedCatalog("const ROWS = `\n/bad|new-kind|both\n`;\n"), + ).toBeNull(); + expect( + parseCommittedCatalog("const ROWS = `\n|menu|both\n`;\n"), + ).toBeNull(); + expect( + parseCommittedCatalog( + "const ROWS = `\n/extra|menu|both|pkg|cond|syscap|unexpected\n`;\n", + ), + ).toBeNull(); + expect( + parseCommittedCatalog( + "const ROWS = `\n/duplicate|menu|both\n/duplicate|menu|both\n`;\n", + ), + ).toBeNull(); + + const report = buildDriftReport(null, [row("/one", "menu", "published")]); + expect(report.startsWith(`${DRIFT_REPORT_SENTINEL}\n`)).toBe(true); + expect(report).toContain( + "Fresh catalog has 1 paths; committed file could not be parsed (binary diff).", + ); + expect(report).toContain("git diff src/explain/catalog.ts"); + }); + + test("the QA fallback guards against duplicate reports with the sentinel", async () => { + const workflow = await readFile( + join(import.meta.dir, "..", "..", ".github", "workflows", "qa.yaml"), + "utf8", + ); + expect(workflow).toContain( + `grep -qF "${DRIFT_REPORT_SENTINEL}" "$GITHUB_STEP_SUMMARY"`, + ); + }); + + test("appends to the Actions summary and treats publication as best-effort", async () => { + const dir = await mkdtemp(join(tmpdir(), "centrs-catalog-drift-")); + try { + const summary = join(dir, "summary.md"); + await writeFile(summary, "earlier step\n"); + + expect(appendDriftReportToSummary("drift report", summary)).toBe(true); + expect(await readFile(summary, "utf8")).toBe( + "earlier step\ndrift report\n", + ); + expect(appendDriftReportToSummary("ignored", undefined)).toBe(false); + expect( + appendDriftReportToSummary( + "ignored", + join(dir, "missing", "summary.md"), + ), + ).toBe(false); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +});