diff --git a/.changeset/eager-mice-caper.md b/.changeset/eager-mice-caper.md new file mode 100644 index 000000000..d383f3ce3 --- /dev/null +++ b/.changeset/eager-mice-caper.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2656 +--- +**`api-coverage` now ships the #2366 coverage-matrix fix** — the tracked `gsd-core/bin/lib/api-coverage.cjs` build artifact had drifted four days behind `src/api-coverage.cts`, so the module that actually ships still parsed non-coverage tables as data, mishandled multi-section matrices with repeated headers, and failed to parse `**OPT-OUT**`. Regenerated, plus a new `lint:generated-sync` check that fails when any tracked compiled artifact no longer matches its source. Also prunes two stale entries from the `no-phantom-issue-refs` guard: GitHub numbers issues and PRs from one shared counter, so both had since become real merged PRs, and the guard was rejecting accurate citations of them. (#2653) diff --git a/gsd-core/bin/lib/api-coverage.cjs b/gsd-core/bin/lib/api-coverage.cjs index e7e2aed6b..92fdc5b71 100644 --- a/gsd-core/bin/lib/api-coverage.cjs +++ b/gsd-core/bin/lib/api-coverage.cjs @@ -562,13 +562,19 @@ function parseCoverageMatrix(text) { } return out; } - // (2) markdown table — collect table rows whose decision column parses. + // (2) markdown table — collect rows from coverage matrix tables only (#2366). + // Track whether we are inside a recognized coverage matrix (after a header + // row, before a non-pipe line ends the table). This prevents summary tables + // elsewhere in the file from being parsed as data (#2366 bug 1) and allows + // multi-section matrices with repeated headers (#2366 bug 2). const lines = src.split('\n'); - let sawHeader = false; + let inMatrix = false; for (const line of lines) { const trimmed = line.trim(); - if (!trimmed.startsWith('|')) + if (!trimmed.startsWith('|')) { + inMatrix = false; continue; + } const cells = trimmed.slice(1, trimmed.endsWith('|') ? -1 : trimmed.length).split('|'); if (cells.length < 2) continue; @@ -577,13 +583,21 @@ function parseCoverageMatrix(text) { // is not mistaken for a separator. if (cleaned.every((c) => /^:?-{3,}:?$/.test(c))) continue; - const decisionCell = (cleaned[1] || '').toUpperCase(); - // header detection - if (!sawHeader && cleaned[0].toLowerCase() === 'capability') { - sawHeader = true; - out.format = 'table'; + // Strip markdown emphasis (**, *, __, _, `) from the decision cell before + // comparison so **OPT-OUT** parses correctly (#2366 bug 3). + const decisionCell = (cleaned[1] || '').replace(/[*_`]/g, '').trim().toUpperCase(); + // header detection — recognized by 'capability' in column 0; allows multiple + // headers for multi-section matrices (#2366 bug 2). + if (cleaned[0].toLowerCase() === 'capability') { + inMatrix = true; + if (out.format === 'none') + out.format = 'table'; continue; } + // Only parse data rows from inside a recognized coverage matrix table. + // A pipe-table outside the matrix (e.g., a summary table) is ignored (#2366 bug 1). + if (!inMatrix) + continue; if (!VALID_DECISIONS.has(decisionCell)) { // A row that otherwise looks like data (≥3 cells, non-empty capability) // but carries a malformed decision is a real error, not a row to skip diff --git a/package.json b/package.json index 02b8daccf..1000550c3 100644 --- a/package.json +++ b/package.json @@ -111,7 +111,7 @@ "lint:test-file-count": "node scripts/lint-test-file-count.cjs", "lint:pr-checks": "node scripts/lint-pr-check-project-dir.cjs", "lint:changeset": "node scripts/changeset/lint.cjs", - "lint:generated-sync": "node scripts/gen-capability-registry.cjs --check && node scripts/gen-loop-host-contract.cjs --check && node scripts/gen-capability-matrix.cjs --check && node scripts/sync-manifest-versions.cjs --check && node scripts/gen-inventory-manifest.cjs --check && node scripts/generate-package-identity.cjs --check && node scripts/gen-plugin-skills.cjs --check && node scripts/gen-registry.cjs --check && node scripts/gen-adr-index.cjs --check && node scripts/check-glossary-refs.cjs --check", + "lint:generated-sync": "node scripts/gen-capability-registry.cjs --check && node scripts/gen-loop-host-contract.cjs --check && node scripts/gen-capability-matrix.cjs --check && node scripts/sync-manifest-versions.cjs --check && node scripts/gen-inventory-manifest.cjs --check && node scripts/generate-package-identity.cjs --check && node scripts/gen-plugin-skills.cjs --check && node scripts/gen-registry.cjs --check && node scripts/gen-adr-index.cjs --check && node scripts/check-glossary-refs.cjs --check && node scripts/lint-compiled-artifact-sync.cjs --check", "lint:docs": "node scripts/lint-docs-required.cjs", "lint:legacy-name": "node scripts/lint-legacy-dir-name.cjs", "ci:test-scope": "node scripts/ci-test-scope.cjs", diff --git a/scripts/lint-compiled-artifact-sync.cjs b/scripts/lint-compiled-artifact-sync.cjs new file mode 100644 index 000000000..60d4fa7b6 --- /dev/null +++ b/scripts/lint-compiled-artifact-sync.cjs @@ -0,0 +1,146 @@ +#!/usr/bin/env node +/** + * lint-compiled-artifact-sync — fail when a *tracked* compiled artifact under + * gsd-core/bin/lib/ has drifted from its src/*.cts source. + * + * ADR-457 compiles src/*.cts to gsd-core/bin/lib/*.cjs at build time and expects + * those artifacts to be gitignored. Most are (see .gitignore). A handful are + * still tracked because the migration that moved the module into src/ did not + * also add the emitted .cjs to .gitignore. While a compiled artifact remains + * tracked, the committed bytes are what ships to anyone who reads the repo + * without building — so they must match the source. + * + * #2653: gsd-core/bin/lib/api-coverage.cjs sat four days behind its .cts after + * PR #2551 changed the source without regenerating the artifact, shipping a + * module that silently lacked the entire #2366 fix while CI stayed green. + * + * This check is deliberately regime-agnostic: it asserts a property of whatever + * is tracked right now. If the remaining artifacts are later untracked and + * gitignored (the ADR-457 end state), the tracked set becomes empty and this + * script passes trivially — no edit required. + * + * Usage: node scripts/lint-compiled-artifact-sync.cjs [--check] + * Exit 0 when every tracked artifact matches a fresh compile; 1 otherwise. + */ + +'use strict'; + +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const REPO_ROOT = path.resolve(__dirname, '..'); +const LIB_DIR = path.join('gsd-core', 'bin', 'lib'); +const SRC_DIR = 'src'; + +/** Frozen reason codes so tests assert on structure, not prose. */ +const REASON = Object.freeze({ + OK: 'ok_artifacts_in_sync', + DRIFTED: 'fail_artifact_drifted', + BUILD_FAILED: 'fail_build_failed', + MISSING_EMIT: 'fail_missing_emit', +}); + +function git(args) { + return execFileSync('git', args, { cwd: REPO_ROOT, encoding: 'utf8' }); +} + +/** + * Tracked .cjs files under gsd-core/bin/lib/ that have a matching src/*.cts. + * Uses `git ls-files` so the set is derived from what git actually tracks + * rather than a hand-maintained list that could itself drift. + */ +function trackedCompiledArtifacts() { + const tracked = git(['ls-files', LIB_DIR]).split('\n').filter((l) => l.endsWith('.cjs')); + const out = []; + for (const rel of tracked) { + const stem = path.basename(rel, '.cjs'); + const subdir = path.dirname(path.relative(LIB_DIR, rel)); + const srcRel = path.join(SRC_DIR, subdir === '.' ? '' : subdir, `${stem}.cts`); + if (fs.existsSync(path.join(REPO_ROOT, srcRel))) out.push({ artifact: rel, source: srcRel }); + } + return out.sort((a, b) => (a.artifact < b.artifact ? -1 : a.artifact > b.artifact ? 1 : 0)); +} + +/** Compile the whole project to a throwaway outDir so the work tree is untouched. */ +function compileToTemp() { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-artifact-sync-')); + try { + execFileSync( + process.execPath, + [ + path.join(REPO_ROOT, 'node_modules', 'typescript', 'bin', 'tsc'), + '-p', path.join(REPO_ROOT, 'tsconfig.build.json'), + '--outDir', tmp, + // A throwaway outDir must not reuse the in-tree incremental state, or + // tsc skips emit for files it believes are already current. + '--incremental', 'false', + '--tsBuildInfoFile', 'null', + ], + { cwd: REPO_ROOT, encoding: 'utf8', stdio: 'pipe' }, + ); + return { ok: true, dir: tmp }; + } catch (err) { + fs.rmSync(tmp, { recursive: true, force: true }); + const detail = [err.stdout, err.stderr].filter(Boolean).join('\n').trim(); + return { ok: false, detail }; + } +} + +function main() { + const pairs = trackedCompiledArtifacts(); + if (pairs.length === 0) { + console.log('ok compiled-artifact-sync: no tracked compiled artifacts (ADR-457 end state)'); + return 0; + } + + const build = compileToTemp(); + if (!build.ok) { + console.error(`FAIL compiled-artifact-sync: ${REASON.BUILD_FAILED}`); + console.error(build.detail); + return 1; + } + + const drifted = []; + const missing = []; + try { + for (const { artifact, source } of pairs) { + const fresh = path.join(build.dir, path.relative(LIB_DIR, artifact)); + if (!fs.existsSync(fresh)) { missing.push({ artifact, source }); continue; } + const a = fs.readFileSync(path.join(REPO_ROOT, artifact)); + const b = fs.readFileSync(fresh); + if (!a.equals(b)) drifted.push({ artifact, source, committed: a.length, expected: b.length }); + } + } finally { + fs.rmSync(build.dir, { recursive: true, force: true }); + } + + if (missing.length > 0) { + console.error(`FAIL compiled-artifact-sync: ${REASON.MISSING_EMIT}`); + for (const m of missing) console.error(` ${m.artifact} — no emit produced from ${m.source}`); + return 1; + } + + if (drifted.length > 0) { + console.error(`FAIL compiled-artifact-sync: ${REASON.DRIFTED}`); + for (const d of drifted) { + console.error(` ${d.artifact} (${d.committed} bytes) != compile of ${d.source} (${d.expected} bytes)`); + } + console.error(''); + console.error('The committed artifact is what ships to anyone reading the repo without'); + console.error('building, so it must match its source. Fix with:'); + console.error(' npm run build:lib && git add ' + drifted.map((d) => d.artifact).join(' ')); + console.error(''); + console.error('Alternatively, per ADR-457 these artifacts are meant to be gitignored —'); + console.error('untracking them (git rm --cached + .gitignore) also resolves this.'); + return 1; + } + + console.log(`ok compiled-artifact-sync: ${pairs.length} tracked artifact(s) match their source`); + return 0; +} + +if (require.main === module) process.exitCode = main(); + +module.exports = { REASON, trackedCompiledArtifacts }; diff --git a/tests/no-phantom-issue-refs.test.cjs b/tests/no-phantom-issue-refs.test.cjs index bf862e5b0..fc56fcffd 100644 --- a/tests/no-phantom-issue-refs.test.cjs +++ b/tests/no-phantom-issue-refs.test.cjs @@ -1,9 +1,20 @@ // allow-test-rule: runtime-contract-is-the-product (see #1073) — this guard asserts the // ABSENCE of phantom pre-migration issue references in repo text (docs, tests, // workflows). The file *content* is the product surface here (#1073): dangling -// refs like #2551/#3182 that don't exist in open-gsd/gsd-core (highest real -// issue is in the low thousands of the redux repo, not here) mislead triage and -// manufacture phantom blockers. This test fails CI if such a ref is reintroduced. +// refs that don't exist in open-gsd/gsd-core mislead triage and manufacture +// phantom blockers. This test fails CI if such a ref is reintroduced. +// +// MAINTENANCE — this list ROTS and must be pruned (#2653). +// GitHub numbers issues and pull requests from one shared counter, so every +// entry below is phantom only until this repo's counter reaches it. Two of the +// original three have already gone real: +// 2551 -> merged PR "fix(#2366): scope parseCoverageMatrix to recognized coverage tables" +// 2361 -> merged PR "docs(#2357): fix Registry Discussions category name and state its format" +// While they remained listed this guard rejected legitimate citations of those +// PRs — it fired on a build-artifact-drift fix that named PR 2551 as the +// provenance of the drift, which is exactly the sort of accurate reference the +// repo wants. Before adding a number, confirm it sits above the current +// counter; once the counter passes an entry, delete it. 'use strict'; @@ -17,9 +28,13 @@ const ROOT = path.resolve(__dirname, '..'); // Phantom pre-migration (get-shit-done-redux) issue numbers with NO equivalent // in open-gsd/gsd-core. Matched only with a leading '#' or in an issues/ URL so -// SSH key patterns like `id_ed25519` (which contain the digits "2551") are NOT +// digit-bearing strings like an `id_ed25519` SSH key fingerprint are NOT // false-positives. -const PHANTOM = ['2551', '3182', '2361']; +// +// Re-verified against the live repo on 2026-07-25, when the shared issue/PR +// counter stood at 2654: 3182 is still a 404; 2551 and 2361 now resolve to +// merged PRs and were removed (#2653). +const PHANTOM = ['3182']; const REF_RE = new RegExp( '(?:#(?:' + PHANTOM.join('|') + ')\\b)|(?:issues/(?:' + PHANTOM.join('|') + ')\\b)', );