fix(validate): type-check list and boolean fields and strip trailing comments in scalars

site/check-links.mjs

@@ -21,10 +21,9 @@
 import fs from "node:fs";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
-import { maintenanceFromRepo } from "./checks.mjs";
 import { parseFrontmatter } from "./lib/frontmatter.mjs";
 import { listEntryFiles } from "./lib/entry-files.mjs";
-import { classify, repoOf, reportUrl } from "./lib/link-check.mjs";
+import { checkMaintenance, classify, parseStaleDays, reportUrl } from "./lib/link-check.mjs";
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 const ROOT = path.resolve(__dirname, "..");
@@ -82,48 +81,22 @@ async function run(entries) {
 }
 
 /**
- * GitHub sources whose repository state and `maintenance` field disagree.
- * One API call per github.com repository, one at a time: a few dozen calls,
- * well inside the limit with a token. An API failure is listed, not guessed.
+ * The GitHub maintenance check now lives in ./lib/link-check.mjs
+ * (`checkMaintenance`) so its failure classification is tested: it returns
+ * `{ differs, skipped }`, and API failures are never counted as problems.
  */
-async function checkMaintenance(entries) {
-  const headers = { "user-agent": UA, accept: "application/vnd.github+json" };
-  if (process.env.GITHUB_TOKEN) headers.authorization = `Bearer ${process.env.GITHUB_TOKEN}`;
-  const out = [];
-  for (const e of entries) {
-    let host;
-    try {
-      host = new URL(e.url).hostname;
-    } catch {
-      continue;
-    }
-    if (host !== "github.com") continue;
-    const repo = repoOf(e.url);
-    if (!repo.includes("/")) continue;
-    try {
-      const res = await fetch(`https://api.github.com/repos/${repo}`, { headers, signal: AbortSignal.timeout(TIMEOUT_MS) });
-      if (!res.ok) {
-        out.push({ ...e, detail: `GitHub API HTTP ${res.status} for ${repo}` });
-        if (res.status === 403 || res.status === 429) break; // rate limited: the rest would fail too
-        continue;
-      }
-      const data = await res.json();
-      const want = maintenanceFromRepo(data);
-      if (want !== e.maintenance) {
-        const state = data.archived ? "archived" : `last push ${String(data.pushed_at).slice(0, 10)}`;
-        out.push({ ...e, detail: `${repo} is ${state}; maintenance is ${e.maintenance || "unset"}, expected ${want || "unset"}` });
-      }
-    } catch (err) {
-      out.push({ ...e, detail: `GitHub API ${err.name || "error"} for ${repo}` });
-    }
-  }
-  return out;
-}
 
-const staleDays = Number(arg("--stale-days", "365"));
+let staleDays;
+try {
+  staleDays = parseStaleDays(arg("--stale-days", "365"));
+} catch (err) {
+  console.error(err.message);
+  console.error("usage: node site/check-links.mjs [--report <file.md>] [--stale-days 365]");
+  process.exit(2);
+}
 const entries = loadEntries().filter((e) => e.status !== "deprecated");
 const results = await run(entries);
-const maintenance = await checkMaintenance(entries);
+const { differs: maintenanceDiffers, skipped: maintenanceSkipped } = await checkMaintenance(entries, { userAgent: UA, timeoutMs: TIMEOUT_MS });
 const cutoff = new Date(Date.now() - staleDays * 86400000).toISOString().slice(0, 10);
 const rows = entries.map((e, i) => ({ ...e, ...results[i] }));
 const pick = (kind) => rows.filter((r) => r.kind === kind);
@@ -133,7 +106,7 @@ const line = (r) => `- [ ] [\`${r.id}\`](${REPO}/blob/main/${r.rel}): ${reportUr
 const dead = pick("dead");
 const moved = pick("moved");
 const blocked = pick("blocked");
-const problems = dead.length + moved.length + stale.length + maintenance.length;
+const problems = dead.length + moved.length + stale.length + maintenanceDiffers.length;
 const date = new Date().toISOString().slice(0, 10);
 const report = [
   `Link check of ${entries.length} active and needs-review entries, run ${date}.`,
@@ -152,9 +125,13 @@ const report = [
   "",
   ...(stale.length ? stale.map((e) => `- [ ] [\`${e.id}\`](${REPO}/blob/main/${e.rel}): verified ${e.verified}`) : ["None."]),
   "",
-  `### GitHub repository state differs from \`maintenance\` (${maintenance.length})`,
+  `### GitHub repository state differs from \`maintenance\` (${maintenanceDiffers.length})`,
+  "",
+  ...(maintenanceDiffers.length ? maintenanceDiffers.map(line) : ["None."]),
+  "",
+  `### Could not check (GitHub API failures, ${maintenanceSkipped.length})`,
   "",
-  ...(maintenance.length ? maintenance.map(line) : ["None."]),
+  ...(maintenanceSkipped.length ? maintenanceSkipped.map(line) : ["None."]),
   "",
   `<details><summary>Refused the checker (${blocked.length}): usually bot blocking, check by hand</summary>`,
   "",
@@ -168,4 +145,4 @@ const reportPath = arg("--report", null);
 if (reportPath) fs.writeFileSync(reportPath, report);
 else process.stdout.write(report);
 if (process.env.GITHUB_OUTPUT) fs.appendFileSync(process.env.GITHUB_OUTPUT, `problems=${problems}\n`);
-console.error(`${entries.length} checked: ${dead.length} dead, ${moved.length} moved, ${stale.length} stale, ${maintenance.length} maintenance, ${blocked.length} blocked`);
+console.error(`${entries.length} checked: ${dead.length} dead, ${moved.length} moved, ${stale.length} stale, ${maintenanceDiffers.length} maintenance, ${maintenanceSkipped.length} API-unchecked, ${blocked.length} blocked`);

site/checks.mjs

@@ -30,6 +30,45 @@ export function evidenceDates(ev) {
 }
 
 /** Generic hosts and distributors that are never a rights holder. */
+export const LIST_FIELDS = ["subcategories", "formats", "tags", "hardware_tags"];
+
+/**
+ * The frontmatter fields that are lists must actually be lists. The parser
+ * used to turn `tags: [a, b]   # note` into the string "[a, b]   # note"
+ * and every downstream check silently fell back to `[]`, so the entry was
+ * built with no tags at all; a quoted `commercial: "true"` likewise passed
+ * the enum check as a string and rendered as "commercial ?" (#79).
+ */
+export function checkListTypes(rel, meta, fields = LIST_FIELDS) {
+  const errors = [];
+  for (const field of fields) {
+    const value = meta[field];
+    if (value === undefined || value === null || empty(value)) continue;
+    if (!Array.isArray(value)) {
+      const shown = typeof value === "string" ? `"${value}"` : String(value);
+      errors.push(`${rel} ${field} must be a list ([a, b]), got the ${typeof value} ${shown}`);
+    }
+  }
+  return errors;
+}
+
+/**
+ * `commercial` and `attribution_required` are booleans; the documented
+ * string answers are listed per key. A quoted "true" is a string and is
+ * rejected rather than accepted as an answer (#79).
+ */
+export function checkBooleanField(rel, meta, key, stringAnswers) {
+  const value = meta[key];
+  if (value === undefined || value === null || empty(value)) return [];
+  if (typeof value === "boolean") return [];
+  const answer = String(value).toLowerCase();
+  if (stringAnswers.has(answer)) return [];
+  const allowed = `true or false${stringAnswers.size ? ` or ${[...stringAnswers].join("|")}` : ""}`;
+  return [`${rel} ${key} must be ${allowed}, got the string "${value}"`];
+}
+
+/** Generic hosts that host many rights holders, so a name like "GitHub" is
+ * not a publisher. See checkPublisherConsistency for how it is used. */
 export const GENERIC_HOSTS = new Set([
   "github",
   "github.com",

site/checks.test.mjs

@@ -12,6 +12,7 @@ import { fileURLToPath } from "node:url";
 import {
   checkActiveIsSettled,
   checkAttributionConsistency,
+  checkBooleanField,
   checkCategoryReadmeRows,
   checkCategorySets,
   checkCountTables,
@@ -23,6 +24,7 @@ import {
   evidenceDates,
   markdownAnchors,
   checkLicenseVocabulary,
+  checkListTypes,
   checkMaintenance,
   maintenanceFromRepo,
   checkPublisherConsistency,
@@ -748,6 +750,27 @@ accepts(
   formatVocab.formats.filter((f) => Object.prototype.hasOwnProperty.call(aliases.formats, f))
 );
 
+/* field types (#79) ------------------------------------------------------- */
+
+rejects("#79 string tags are a type error", checkListTypes("bad.md", { tags: "[a, b]   # note" }), "must be a list");
+rejects("#79 string formats are a type error", checkListTypes("bad.md", { formats: "PNG" }), "must be a list");
+accepts("#79 list tags pass", checkListTypes("good.md", { tags: ["low-poly"] }));
+accepts("#79 absent optional list passes", checkListTypes("good.md", {}));
+accepts("#79 empty list passes (emptiness is a separate rule)", checkListTypes("good.md", { tags: [] }));
+rejects(
+  "#79 quoted commercial string is rejected",
+  checkBooleanField("bad.md", { commercial: "true" }, "commercial", new Set(["unknown", "varies"])),
+  'must be true or false or unknown|varies, got the string "true"'
+);
+accepts("#79 boolean commercial passes", checkBooleanField("good.md", { commercial: true }, "commercial", new Set(["unknown", "varies"])));
+accepts("#79 string varies passes", checkBooleanField("good.md", { commercial: "varies" }, "commercial", new Set(["unknown", "varies"])));
+accepts("#79 string unknown passes for attribution", checkBooleanField("good.md", { attribution_required: "unknown" }, "attribution_required", new Set(["unknown"])));
+rejects(
+  "#79 quoted attribution string is rejected",
+  checkBooleanField("bad.md", { attribution_required: "false" }, "attribution_required", new Set(["unknown"])),
+  'must be true or false or unknown'
+);
+
 /* ----------------------------------------------------------------------- */
 if (failures.length) {
   console.error(`checks.test failed (${failures.length}):`);

site/lib/frontmatter.mjs

@@ -35,8 +35,28 @@ function splitFlowList(inner) {
   return parts.map((p) => unquoteScalar(p.trim()));
 }
 
+/**
+ * Removes an unquoted trailing `# comment` — YAML needs a space before the
+ * `#` — so a template line like `tags: [a, b]   # note` parses as the list
+ * and not as the string "[a, b]   # note" (#79). A `#` inside quotes, or
+ * with no space before it (a URL fragment), is kept.
+ */
+function stripTrailingComment(raw) {
+  let inSingle = false;
+  let inDouble = false;
+  for (let i = 0; i < raw.length; i += 1) {
+    const ch = raw[i];
+    if (ch === "'" && !inDouble) inSingle = !inSingle;
+    else if (ch === '"' && !inSingle) inDouble = !inDouble;
+    else if (ch === "#" && !inSingle && !inDouble && i > 0 && (raw[i - 1] === " " || raw[i - 1] === "\t")) {
+      return raw.slice(0, i).trimEnd();
+    }
+  }
+  return raw;
+}
+
 export function parseScalar(raw) {
-  const v = raw.trim();
+  const v = stripTrailingComment(raw).trim();
   if (v === "true") return true;
   if (v === "false") return false;
   if (v === "null" || v === "~" || v === "") return null;

site/lib/lib.test.mjs

@@ -469,7 +469,7 @@ has("recent window wording counts the build day", fp, "Checked on 2026-09-25 or
 has("empty recent window wording counts the build day", freshnessPageHtml({ stats: freshnessStats([fe("a", "Alpha", 90)], fNow), site, stamp: "2026-09-25", total: 1, hasCard: false }), "None checked on 2026-09-25 or in the 30 days before it.");
 
 /* frontmatter -------------------------------------------------------------- */
-import { evidenceSection, parseFrontmatter, plainText, summaryFromBody } from "./frontmatter.mjs";
+import { evidenceSection, parseFrontmatter, parseScalar, plainText, summaryFromBody } from "./frontmatter.mjs";
 import { latestAllowedDate } from "./shared.mjs";
 const fm = (lines, body = "Body.\n") => parseFrontmatter(`---\n${lines.join("\n")}\n---\n${body}`);
 const f1 = fm([
@@ -492,6 +492,24 @@ eq("frontmatter with CRLF line endings", parseFrontmatter("---\r\nid: a\r\nname:
 has("frontmatter rejects a duplicate key", fm(["id: a", "id: b"]).errors.join(), 'line 3: key "id" is given twice');
 has("frontmatter rejects a line that is not key: value", fm(["id: a", "just words"]).errors.join(), 'line 3: "just words" is not a key: value line');
 eq("no frontmatter is null", parseFrontmatter("# Title\n"), null);
+
+/* frontmatter: trailing comments (#79) ------------------------------------ */
+
+eq("scalar keeps a url fragment", parseScalar("https://example.com/#anchor"), "https://example.com/#anchor");
+eq("scalar strips a trailing comment", JSON.stringify(parseScalar("[low-poly, modular]   # describe the content")), JSON.stringify(["low-poly", "modular"]));
+eq("scalar strips a comment on a boolean", parseScalar("true   # set active once"), true);
+eq("scalar keeps a quoted hash", parseScalar('"a # not a comment"'), "a # not a comment");
+eq("scalar strips only outside quotes", parseScalar('"http://x"   # note'), "http://x");
+eq("scalar keeps a hash with no space before it", parseScalar("[a,b]#note"), "[a,b]#note");
+
+const fComment = fm([
+  "id: a",
+  "tags: [low-poly, modular]   # describe the content; never the licence or the publisher",
+  "commercial: true   # yes",
+]);
+eq("template tags line parses as a list", JSON.stringify(fComment.meta.tags), JSON.stringify(["low-poly", "modular"]));
+eq("commented boolean still parses", fComment.meta.commercial, true);
+eq("template tags line has no errors", fComment.errors.length, 0);
 eq("evidence stops at the next heading, whatever its letter", evidenceSection("## Evidence\n\n- e (2026-01-01)\n\n## Extras\n\n- x"), "## Evidence\n\n- e (2026-01-01)\n");
 eq("summary keeps underscores", summaryFromBody("Split by Rig_Medium and **KHR_draco**.\n\n## Notes"), "Split by Rig_Medium and KHR_draco.");
 eq("summary keeps C# and link text", summaryFromBody("# T\n\nA [C#](x.md) library, `code`, *em*."), "A C# library, code, em.");
@@ -683,6 +701,68 @@ const neText = newEntryText("id: example\ncategory: 3d\nverified: 2026-07-19   #
 eq("new-entry text", neText, "id: my-pack\ncategory: 2d\nverified: 2026-10-04\nstatus: needs-review\n");
 eq("template copy starts needs-review", newEntryText(nodeFs.readFileSync(new URL("../../catalog/TEMPLATE.md", import.meta.url), "utf8"), { id: "a", category: "2d", today: "2026-10-04" }).split(/\r?\n/).includes("status: needs-review"), true);
 
+/* check-links helpers (#82) ------------------------------------------------ */
+import { checkMaintenance, parseStaleDays } from "./link-check.mjs";
+
+eq("stale-days: whole number", parseStaleDays("365"), 365);
+eq("stale-days: zero allowed", parseStaleDays("0"), 0);
+let staleThrew = "";
+try { parseStaleDays("abc"); } catch (e) { staleThrew = e.message; }
+has("stale-days: non-number rejected", staleThrew, "whole number");
+staleThrew = "";
+try { parseStaleDays(undefined); } catch (e) { staleThrew = e.message; }
+has("stale-days: missing value rejected", staleThrew, "undefined");
+staleThrew = "";
+try { parseStaleDays("-5"); } catch (e) { staleThrew = e.message; }
+has("stale-days: negative rejected", staleThrew, "whole number");
+staleThrew = "";
+try { parseStaleDays("3.5"); } catch (e) { staleThrew = e.message; }
+has("stale-days: fraction rejected", staleThrew, "whole number");
+
+const ghEntry = (id, maintenance) => ({ id, url: `https://github.com/example/${id}`, maintenance, rel: `catalog/2d/${id}.md` });
+const ghRes = (status, data) => ({ ok: status >= 200 && status < 300, status, json: async () => data });
+
+{
+  const { differs, skipped } = await checkMaintenance([ghEntry("a", null)], { fetchImpl: async () => ghRes(500, {}) });
+  eq("maintenance: 5xx not a problem", differs.length, 0);
+  eq("maintenance: 5xx skipped", skipped.length, 1);
+  has("maintenance: 5xx detail", skipped[0].detail, "HTTP 500");
+}
+{
+  const { differs, skipped } = await checkMaintenance([ghEntry("a", null)], {
+    fetchImpl: async () => ghRes(200, { archived: true, pushed_at: "2026-01-01T00:00:00Z" }),
+  });
+  eq("maintenance: mismatch found", differs.length, 1);
+  eq("maintenance: mismatch only", skipped.length, 0);
+  has("maintenance: mismatch detail", differs[0].detail, "archived");
+}
+{
+  let calls = 0;
+  const { differs, skipped } = await checkMaintenance([ghEntry("a", null), ghEntry("b", null), ghEntry("c", null)], {
+    fetchImpl: async () => { calls += 1; return ghRes(403, {}); },
+  });
+  eq("maintenance: rate limit stops the loop", calls, 1);
+  eq("maintenance: rest reported skipped", skipped.length, 3);
+  has("maintenance: skipped reason", skipped[2].detail, "not checked");
+  eq("maintenance: rate limit not a problem", differs.length, 0);
+}
+{
+  const { differs, skipped } = await checkMaintenance([ghEntry("a", null)], {
+    fetchImpl: async () => { throw new Error("timeout"); },
+  });
+  eq("maintenance: fetch error skipped", skipped.length, 1);
+  has("maintenance: fetch error detail", skipped[0].detail, "GitHub API");
+  eq("maintenance: fetch error not a problem", differs.length, 0);
+}
+{
+  let calls = 0;
+  const { differs, skipped } = await checkMaintenance([{ id: "x", url: "https://example.com/a", maintenance: null, rel: "x" }], {
+    fetchImpl: async () => { calls += 1; return ghRes(200, {}); },
+  });
+  eq("maintenance: non-github ignored", calls, 0);
+  eq("maintenance: non-github no rows", differs.length + skipped.length, 0);
+}
+
 /* report (keep last) ------------------------------------------------------ */
 if (failures.length) {
   console.error(`lib.test failed (${failures.length}):`);