From acc1a7abd6cd3e051eaf9ef949dbe86a6b8ac4f1 Mon Sep 17 00:00:00 2001 From: sim Date: Thu, 13 Aug 2026 01:53:20 -0400 Subject: [PATCH] feat(#3309): add health-diagnostic rule-table lint guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enforces ADR-3180 §8.2's 1:1 rule-code invariant (every code unique, every severity a property of the Rule) and §8.5's fixture-proof invariant (every code has a describe()/test() block naming it, verified statically against tests/health-diagnostic-rules/*.test.cjs and tests/health-diagnostic.test.cjs) for the new RULES table. Adapted from the design doc's original plan of separate tests/fixtures/health-diagnostic/.* files: implementation used inline temp-dir fixtures instead (mirrors tests/planning-snapshot.test.cjs), so coverage is checked statically against test-file structure, mirroring lint-fix-has-regression-test.cjs's house style. Wired into lint:ci adjacent to lint-planning-snapshot-bypass-drift.cjs, its closest sibling. Passes clean against the real tree: 31 codes, all unique, all covered. --- package.json | 2 +- scripts/lint-health-diagnostic-rule-table.cjs | 250 ++++++++++++++++++ ...lint-health-diagnostic-rule-table.test.cjs | 164 ++++++++++++ 3 files changed, 415 insertions(+), 1 deletion(-) create mode 100644 scripts/lint-health-diagnostic-rule-table.cjs create mode 100644 tests/lint-health-diagnostic-rule-table.test.cjs diff --git a/package.json b/package.json index 0963f9798..2ca21c99a 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "lint:table-schema-drift": "node scripts/lint-table-schema-drift.cjs", "lint:frontmatter-scalar-broad-grep": "node scripts/lint-frontmatter-scalar-broad-grep.cjs", "lint:removed-but-needed": "node scripts/lint-removed-but-needed.cjs", - "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs", + "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs", "lint:allow-test-rule-refs": "node scripts/lint-allow-test-rule-refs.cjs", "lint:regression-names": "node scripts/lint-regression-test-names.cjs", "lint:descriptions": "node scripts/lint-descriptions.cjs", diff --git a/scripts/lint-health-diagnostic-rule-table.cjs b/scripts/lint-health-diagnostic-rule-table.cjs new file mode 100644 index 000000000..7a302868f --- /dev/null +++ b/scripts/lint-health-diagnostic-rule-table.cjs @@ -0,0 +1,250 @@ +#!/usr/bin/env node +'use strict'; + +/** + * lint-health-diagnostic-rule-table.cjs — gate: enforces ADR-3180 §8.2's 1:1 + * rule-code invariant and §8.5's fixture-proof invariant for + * `src/health-diagnostic.cts`'s RULES table (Phase 11, #3309). + * + * ## What this enforces + * + * 1. (§8.2 rule 1 — 1:1 code invariant) Every `rule.code` in RULES (exported + * from the compiled `gsd-core/bin/lib/health-diagnostic.cjs`) is unique, + * and every rule's `severity` is one of `SEVERITY`'s values. The severity + * check exists only to confirm the compiled artifact was not hand-edited + * to bypass the `Rule.severity` required field TypeScript already + * enforces at compile time — "severity is a property of the RULE, never + * the emit call." + * 2. (§8.5 — fixture-proof invariant) Every code in RULES has a paired test: + * the code string (e.g. `'W001'`) appears as a literal AND within a + * `describe(`/`test(` block whose title also names that exact code, in + * one of the health-diagnostic test files + * (`tests/health-diagnostic-rules/*.test.cjs`, + * `tests/health-diagnostic.test.cjs`). A mere comment/string mention + * outside a titled block does not count as coverage. + * + * Design: .gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md + * ("The lint guard (§8.2 1:1 invariant + §8.5 fixture proof)"). + * + * ## Deviation from the design doc's original plan + * + * The design doc assumed fixtures would live as separate files at + * `tests/fixtures/health-diagnostic/.*`. That did not happen during + * implementation — all 8 rule-group test files + * (`tests/health-diagnostic-rules/*.test.cjs`) build fixtures INLINE via + * real temp directories (`createTempDir()` from `tests/helpers.cjs`) and a + * real, non-mocked `buildPlanningSnapshot(tmpCwd)` call (see + * `tests/health-diagnostic-rules/root-existence.test.cjs`). This guard + * therefore verifies the fixture-proof invariant STATICALLY against the + * test files' own text — mirroring `scripts/lint-fix-has-regression-test.cjs`'s + * house style — rather than dynamically re-running fixture-building code + * this guard does not own. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const { ExitError, runMain } = require('./lib/cli-exit.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const COMPILED_MODULE_REL = 'gsd-core/bin/lib/health-diagnostic.cjs'; +const COMPILED_MODULE_PATH = path.join(REPO_ROOT, COMPILED_MODULE_REL); +const TEST_GROUP_DIR = path.join(REPO_ROOT, 'tests', 'health-diagnostic-rules'); +const SKELETON_TEST_FILE = path.join(REPO_ROOT, 'tests', 'health-diagnostic.test.cjs'); + +// Matches `describe(`/`test(`/`it(` calls whose first argument is a string +// literal, capturing that literal as the block's title. Line/regex-based +// (not full AST) per this repo's existing lint-guard house style +// (scripts/lint-planning-snapshot-bypass-drift.cjs's scanCode precedent). +const TITLED_BLOCK_RE = /\b(describe|test|it)\(\s*(['"`])((?:\\.|(?!\2)[^\\])*)\2/g; + +/** + * Load the compiled health-diagnostic module. Throws a clear ExitError + * (not a raw MODULE_NOT_FOUND) if `npm run build:lib` has not run. + */ +function loadCompiledModule(compiledPath = COMPILED_MODULE_PATH) { + if (!fs.existsSync(compiledPath)) { + throw new ExitError( + 2, + `lint-health-diagnostic-rule-table: compiled artifact not found at ${COMPILED_MODULE_REL}.\n` + + 'Run `npm run build:lib` first.', + ); + } + return require(compiledPath); +} + +/** + * §8.2 rule 1 — 1:1 code invariant: every rule.code is unique, and every + * rule's severity is a member of SEVERITY's values. + * + * @param {Array<{code: string, severity: string}>} rules + * @param {Record} severity SEVERITY export (code -> value) + * @returns {{duplicates: Array<{code: string, count: number}>, badSeverities: Array<{code: string, severity: unknown}>}} + */ +function checkOneToOneInvariant(rules, severity) { + const severityValues = new Set(Object.values(severity)); + const counts = new Map(); + const badSeverities = []; + + for (const rule of rules) { + counts.set(rule.code, (counts.get(rule.code) || 0) + 1); + if (!severityValues.has(rule.severity)) { + badSeverities.push({ code: rule.code, severity: rule.severity }); + } + } + + const duplicates = [...counts.entries()] + .filter(([, count]) => count > 1) + .map(([code, count]) => ({ code, count })); + + return { duplicates, badSeverities }; +} + +/** + * Extracts every `describe(`/`test(`/`it(` block title found in `text`. + * + * @param {string} text + * @returns {string[]} + */ +function extractTitledBlocks(text) { + const titles = []; + TITLED_BLOCK_RE.lastIndex = 0; + let match; + while ((match = TITLED_BLOCK_RE.exec(text)) !== null) { + titles.push(match[3]); + } + return titles; +} + +/** + * True iff `code` appears verbatim, as a whole token, inside at least one of + * `titles`. Whole-token match guards against a shorter code accidentally + * substring-matching inside an unrelated longer token. + * + * @param {string} code + * @param {string[]} titles + */ +function codeAppearsInTitle(code, titles) { + const codeRe = new RegExp(`(?:^|[^A-Za-z0-9])${code}(?:$|[^A-Za-z0-9])`); + return titles.some((title) => codeRe.test(`|${title}|`)); +} + +/** + * Locates every health-diagnostic test file this guard scans for §8.5 + * fixture-proof coverage. + * + * @param {string} repoRoot + * @returns {string[]} absolute paths, sorted + */ +function findHealthDiagnosticTestFiles(repoRoot = REPO_ROOT) { + const groupDir = path.join(repoRoot, 'tests', 'health-diagnostic-rules'); + const files = []; + if (fs.existsSync(groupDir)) { + for (const entry of fs.readdirSync(groupDir)) { + if (entry.endsWith('.test.cjs')) { + files.push(path.join(groupDir, entry)); + } + } + } + const skeletonTestFile = path.join(repoRoot, 'tests', 'health-diagnostic.test.cjs'); + if (fs.existsSync(skeletonTestFile)) { + files.push(skeletonTestFile); + } + return files.sort(); +} + +/** + * §8.5 — fixture-proof invariant: for every code in `rules`, confirm at + * least one test file in `testFiles` has a `describe(`/`test(`/`it(` block + * whose title names that exact code. + * + * @param {Array<{code: string}>} rules + * @param {string[]} testFiles absolute paths to *.test.cjs files to scan + * @returns {{uncovered: string[], testFilesScanned: string[]}} + */ +function checkFixtureProofInvariant(rules, testFiles) { + const allTitles = []; + for (const file of testFiles) { + const text = fs.readFileSync(file, 'utf8'); + allTitles.push(...extractTitledBlocks(text)); + } + + const uncovered = []; + for (const rule of rules) { + if (!codeAppearsInTitle(rule.code, allTitles)) { + uncovered.push(rule.code); + } + } + + return { uncovered, testFilesScanned: testFiles }; +} + +function formatRepoRelative(absPath) { + return path.relative(REPO_ROOT, absPath).split(path.sep).join('/'); +} + +function main() { + const { RULES, SEVERITY } = loadCompiledModule(); + + const { duplicates, badSeverities } = checkOneToOneInvariant(RULES, SEVERITY); + + const testFiles = findHealthDiagnosticTestFiles(REPO_ROOT); + const { uncovered } = checkFixtureProofInvariant(RULES, testFiles); + + const problems = []; + + if (duplicates.length > 0) { + const list = duplicates.map((d) => ` ${d.code} (${d.count} occurrences)`).join('\n'); + problems.push( + `§8.2 rule 1 violated: ${duplicates.length} duplicated rule code(s) in RULES ` + + `(${COMPILED_MODULE_REL}):\n${list}\n` + + ' remedy: codes are append-only and 1:1 with a single Rule — rename or remove the duplicate.', + ); + } + + if (badSeverities.length > 0) { + const list = badSeverities + .map((b) => ` ${b.code}: severity=${JSON.stringify(b.severity)}`) + .join('\n'); + problems.push( + `§8.2 rule 3 violated: ${badSeverities.length} rule(s) with a severity not in SEVERITY's values:\n${list}\n` + + ' remedy: severity is a property of the RULE — set it to SEVERITY.ERROR/WARNING/INFO.', + ); + } + + if (uncovered.length > 0) { + const scannedList = testFiles.map(formatRepoRelative).join('\n '); + problems.push( + `§8.5 violated: ${uncovered.length} rule code(s) with no describe()/test() block naming them ` + + `(a comment or bare string mention does not count):\n ${uncovered.join(', ')}\n\n` + + ` Searched these test files:\n ${scannedList}\n\n` + + ' remedy: add or extend a describe()/test() title in the matching ' + + 'tests/health-diagnostic-rules/.test.cjs file so the block title ' + + `names the code verbatim (e.g. describe('${uncovered[0]} — ...', () => { ... })), ` + + 'and drive the rule to fire against a real fixture built via createTempDir() + buildPlanningSnapshot() ' + + '(see tests/health-diagnostic-rules/root-existence.test.cjs).', + ); + } + + if (problems.length > 0) { + throw new ExitError(1, `${problems.join('\n\n')}\n`); + } + + console.log( + `lint-health-diagnostic-rule-table: PASS — ${RULES.length} rule code(s), all unique, ` + + `all severities valid, all covered by a titled test block across ${testFiles.length} test file(s).`, + ); +} + +runMain(main); + +module.exports = { + loadCompiledModule, + checkOneToOneInvariant, + extractTitledBlocks, + codeAppearsInTitle, + findHealthDiagnosticTestFiles, + checkFixtureProofInvariant, + COMPILED_MODULE_PATH, + TEST_GROUP_DIR, + SKELETON_TEST_FILE, +}; diff --git a/tests/lint-health-diagnostic-rule-table.test.cjs b/tests/lint-health-diagnostic-rule-table.test.cjs new file mode 100644 index 000000000..2d1662750 --- /dev/null +++ b/tests/lint-health-diagnostic-rule-table.test.cjs @@ -0,0 +1,164 @@ +'use strict'; + +/** + * Tests for `scripts/lint-health-diagnostic-rule-table.cjs` — the guard + * enforcing ADR-3180 §8.2's 1:1 rule-code invariant and §8.5's fixture-proof + * invariant for `src/health-diagnostic.cts`'s RULES table (Phase 11, #3309). + * + * Design: .gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md + * ("The lint guard (§8.2 1:1 invariant + §8.5 fixture proof)"). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const guard = require('../scripts/lint-health-diagnostic-rule-table.cjs'); +const { + checkOneToOneInvariant, + checkFixtureProofInvariant, + findHealthDiagnosticTestFiles, +} = guard; + +const FAKE_SEVERITY = Object.freeze({ ERROR: 'error', WARNING: 'warning', INFO: 'info' }); + +function writeTempTestFile(dir, name, content) { + const full = path.join(dir, name); + fs.writeFileSync(full, content); + return full; +} + +// ─── Check 1 — §8.2 rule 1: 1:1 code invariant ───────────────────────────── + +describe('checkOneToOneInvariant (§8.2 rule 1)', () => { + test('flags a duplicated code', () => { + const rules = [ + { code: 'W001', severity: FAKE_SEVERITY.WARNING }, + { code: 'W002', severity: FAKE_SEVERITY.WARNING }, + { code: 'W001', severity: FAKE_SEVERITY.WARNING }, + ]; + + const { duplicates, badSeverities } = checkOneToOneInvariant(rules, FAKE_SEVERITY); + + assert.deepEqual(duplicates, [{ code: 'W001', count: 2 }]); + assert.deepEqual(badSeverities, []); + }); + + test('passes when every code is unique', () => { + const rules = [ + { code: 'W001', severity: FAKE_SEVERITY.WARNING }, + { code: 'W002', severity: FAKE_SEVERITY.ERROR }, + { code: 'W003', severity: FAKE_SEVERITY.INFO }, + ]; + + const { duplicates, badSeverities } = checkOneToOneInvariant(rules, FAKE_SEVERITY); + + assert.deepEqual(duplicates, []); + assert.deepEqual(badSeverities, []); + }); + + test('flags a severity that is not a member of SEVERITY (hand-edited artifact)', () => { + const rules = [ + { code: 'W001', severity: 'critical' }, + { code: 'W002', severity: FAKE_SEVERITY.WARNING }, + ]; + + const { duplicates, badSeverities } = checkOneToOneInvariant(rules, FAKE_SEVERITY); + + assert.deepEqual(duplicates, []); + assert.deepEqual(badSeverities, [{ code: 'W001', severity: 'critical' }]); + }); +}); + +// ─── Check 2 — §8.5: fixture-proof invariant ─────────────────────────────── + +describe('checkFixtureProofInvariant (§8.5)', () => { + test('flags a code with zero mentions anywhere in the scanned test files', (t) => { + const dir = createTempDir('gsd-lint-hd-rt-nomention-'); + t.after(() => cleanup(dir)); + const file = writeTempTestFile( + dir, + 'fake.test.cjs', + "describe('W001 — something', () => { test('fires', () => {}); });\n", + ); + + const rules = [{ code: 'W001' }, { code: 'W999' }]; + const { uncovered } = checkFixtureProofInvariant(rules, [file]); + + assert.deepEqual(uncovered, ['W999']); + }); + + test('flags a code mentioned only in a comment/string outside any describe/test title', (t) => { + const dir = createTempDir('gsd-lint-hd-rt-comment-only-'); + t.after(() => cleanup(dir)); + const file = writeTempTestFile( + dir, + 'fake.test.cjs', + [ + "// W002 is handled elsewhere, see notes", + "const message = 'refers to W002 in a plain string, not a block title';", + "describe('unrelated block', () => { test('does something', () => {}); });", + '', + ].join('\n'), + ); + + const rules = [{ code: 'W002' }]; + const { uncovered } = checkFixtureProofInvariant(rules, [file]); + + assert.deepEqual(uncovered, ['W002']); + }); + + test('passes a code named in a describe() block title', (t) => { + const dir = createTempDir('gsd-lint-hd-rt-titled-'); + t.after(() => cleanup(dir)); + const file = writeTempTestFile( + dir, + 'fake.test.cjs', + "describe('W003 — some finding', () => { test('fires when absent', () => {}); });\n", + ); + + const rules = [{ code: 'W003' }]; + const { uncovered } = checkFixtureProofInvariant(rules, [file]); + + assert.deepEqual(uncovered, []); + }); + + test('passes a code named in a test()-only title (no wrapping describe)', (t) => { + const dir = createTempDir('gsd-lint-hd-rt-test-only-'); + t.after(() => cleanup(dir)); + const file = writeTempTestFile( + dir, + 'fake.test.cjs', + "test('W010 fires on incomplete agent install', () => {});\n", + ); + + const rules = [{ code: 'W010' }]; + const { uncovered } = checkFixtureProofInvariant(rules, [file]); + + assert.deepEqual(uncovered, []); + }); + + test('passes for a real code (W001) against the real tests/ tree', () => { + const testFiles = findHealthDiagnosticTestFiles(); + assert.ok(testFiles.length > 0, 'expected at least one health-diagnostic test file on disk'); + + const { uncovered } = checkFixtureProofInvariant([{ code: 'W001' }], testFiles); + + assert.deepEqual(uncovered, []); + }); +}); + +// ─── findHealthDiagnosticTestFiles ───────────────────────────────────────── + +describe('findHealthDiagnosticTestFiles', () => { + test('finds every *.test.cjs under tests/health-diagnostic-rules/ plus tests/health-diagnostic.test.cjs', () => { + const files = findHealthDiagnosticTestFiles(); + + assert.ok(files.some((f) => f.endsWith('root-existence.test.cjs'))); + assert.ok(files.some((f) => f.endsWith('state-consistency.test.cjs'))); + assert.ok(files.some((f) => f.endsWith(path.join('tests', 'health-diagnostic.test.cjs')))); + }); +});