From b0bd2f7a48278976a82e4baba296ef891e04ff8c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 3 Jul 2026 19:37:14 -0400 Subject: [PATCH] chore: move committed-generated-artifact freshness checks to lint:ci (#2000) gsd-test's build leg runs the full 'npm run build' (which regenerates capability-registry.cjs, loop-host-contract.cjs, package-identity.cjs, etc.), so committed-freshness guards that lived in the unit suite were masked there: gsd-test passed a stale-commit that CI's shard-1/3 test then red-flagged (caught live on PR #1998). The mandated pre-push gate was green on a commit CI correctly flagged. Move the committed-state --check guards into a new 'lint:generated-sync' script wired into lint:ci (the single orchestrated entry point the lint-tests CI job already runs on a build:lib-only tree, so the committed artifacts are checked without regeneration). gsd-test no longer contains these guards, so it can no longer mask them. - package.json: add lint:generated-sync (7 generators --check); wire into lint:ci. - generate-package-identity.cjs: add --check mode (was the only generator without it); no-arg behaviour unchanged (still writes, as build expects). - Remove the committed-freshness guards from the unit suite, keeping all behavioral/structural tests: - capability-registry.test.cjs: drop the --check describe. - loop-host-contract.test.cjs: drop the committed-file staleness test (keep the normalizeLineEndings unit test). - capability-matrix-sync.test.cjs: drop --check + byte-for-byte (keep the architectural content invariants: every cap appears, security ship:pre). - issue-844-manifest-version-sync.test.cjs: drop describe D (--check). - issue-498-package-identity.test.cjs: drop the drift-check test (keep behavioral module-export tests); drop the now-unused render import and its allow-test-rule exemption (allowlist ratcheted 175 -> 174). --- package.json | 3 +- scripts/generate-package-identity.cjs | 29 ++++++++++++++-- .../lint-allow-test-rule-refs.allowlist.json | 1 - tests/capability-matrix-sync.test.cjs | 33 ++++++------------- tests/capability-registry.test.cjs | 17 ---------- tests/issue-498-package-identity.test.cjs | 20 ++++------- .../issue-844-manifest-version-sync.test.cjs | 14 ++------ tests/loop-host-contract.test.cjs | 23 +++---------- 8 files changed, 53 insertions(+), 87 deletions(-) diff --git a/package.json b/package.json index dc509620e..0878f0b2e 100644 --- a/package.json +++ b/package.json @@ -97,7 +97,7 @@ "pretest:coverage": "npm run build:lib && npm run lint:skill-deps", "lint": "eslint . --cache --cache-location node_modules/.cache/eslint/", "lint:fix": "eslint . --fix", - "lint:ci": "npm run lint && npm run lint:skill-deps && 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", + "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", "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", @@ -105,6 +105,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", "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/generate-package-identity.cjs b/scripts/generate-package-identity.cjs index 3506eadf4..1adbddd65 100644 --- a/scripts/generate-package-identity.cjs +++ b/scripts/generate-package-identity.cjs @@ -114,12 +114,37 @@ function render(identity) { function main() { const fs = require('node:fs'); const path = require('node:path'); + const args = process.argv.slice(2); + const check = args.includes('--check'); const pkg = require(path.join(__dirname, '..', 'package.json')); const out = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'package-identity.cjs'); - fs.writeFileSync(out, render(deriveIdentity(pkg))); + const rendered = render(deriveIdentity(pkg)); + if (check) { + // Compare normalized content so a CRLF checkout (Windows, no .gitattributes + // eol rule) does not register as stale. Mirrors the in-process check the + // unit suite used to perform (issue #498) and the convention used by the + // other gen-* --check generators. + const norm = (s) => s.replace(/\r\n/g, '\n'); + let committed = ''; + try { + committed = fs.readFileSync(out, 'utf8'); + } catch (e) { + if (e.code !== 'ENOENT') throw e; + } + if (norm(committed) !== norm(rendered)) { + process.stderr.write('package-identity.cjs is stale — run: node scripts/generate-package-identity.cjs\n'); + return 1; + } + return 0; + } + fs.writeFileSync(out, rendered); process.stdout.write(`wrote ${path.relative(path.join(__dirname, '..'), out)}\n`); + return 0; } -if (require.main === module) main(); +if (require.main === module) { + const code = main(); + if (typeof code === 'number' && code !== 0) process.exitCode = code; +} module.exports = { deriveIdentity, parseRepoSlug, slugifyPackageName, formatManualInstall, render, main }; diff --git a/scripts/lint-allow-test-rule-refs.allowlist.json b/scripts/lint-allow-test-rule-refs.allowlist.json index 8f5070836..af7f01615 100644 --- a/scripts/lint-allow-test-rule-refs.allowlist.json +++ b/scripts/lint-allow-test-rule-refs.allowlist.json @@ -88,7 +88,6 @@ "tests/ios-scaffold-safety.test.cjs :: source-text-is-the-product", "tests/issue-2639-codex-toml-neutralization.test.cjs :: source-text-is-the-product", "tests/issue-429-comment-text-gate.test.cjs :: source-text-is-the-product", - "tests/issue-498-package-identity.test.cjs :: architectural-invariant", "tests/issue-498-update-backup-runtime-dir.test.cjs :: structural assertion on the deployed update.md backup bash;", "tests/issue-57-runtime-install-no-drift.test.cjs :: delegation-presence guard. Catches wholesale removal of the registry", "tests/issue-57-runtime-install-no-drift.test.cjs :: structural guard over bin/install.js source. Behavioral assertions", diff --git a/tests/capability-matrix-sync.test.cjs b/tests/capability-matrix-sync.test.cjs index 0c25087fe..738055ed3 100644 --- a/tests/capability-matrix-sync.test.cjs +++ b/tests/capability-matrix-sync.test.cjs @@ -1,41 +1,28 @@ 'use strict'; /** - * capability-matrix-sync.test.cjs — ADR-1244 Phase 6 (D9) drift guard. + * capability-matrix-sync.test.cjs — ADR-1244 Phase 6 (D9) content invariants. * - * Asserts the committed docs/reference/capability-matrix.md is exactly what - * scripts/gen-capability-matrix.cjs would generate from the current registry. - * If a capability is added/removed or its tier/role/engines/extension-points/ - * hook-kinds change without regenerating the matrix, this fails — the same - * pattern that keeps docs/INVENTORY-MANIFEST.json and capability-registry.cjs honest. + * The committed docs/reference/capability-matrix.md FRESHNESS guard + * (`gen-capability-matrix.cjs --check` + byte-for-byte) moved to + * `npm run lint:generated-sync`, where it runs against the committed file in + * both local and CI lint lanes — instead of being masked by gsd-test's + * `npm run build` leg regenerating artifacts. What remains here are the + * architectural CONTENT invariants the matrix must satisfy regardless of how + * freshness is enforced: every first-party capability appears as a row, and + * the rendered extension points reflect the registry (not placeholders). */ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); const ROOT = path.resolve(__dirname, '..'); -const GENERATOR = path.join(ROOT, 'scripts', 'gen-capability-matrix.cjs'); const MATRIX = path.join(ROOT, 'docs', 'reference', 'capability-matrix.md'); -const { buildMatrix } = require('../scripts/gen-capability-matrix.cjs'); const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); -describe('capability-matrix drift guard (ADR-1244 Phase 6)', () => { - test('the committed matrix is in sync with the registry (`gen-capability-matrix.cjs --check` exits 0)', () => { - // execFileSync throws if the generator exits non-zero (i.e. the committed file is stale). - assert.doesNotThrow(() => { - execFileSync(process.execPath, [GENERATOR, '--check'], { cwd: ROOT, stdio: 'pipe' }); - }, 'committed capability-matrix.md is stale — run: node scripts/gen-capability-matrix.cjs --write'); - }); - - test('buildMatrix(registry) equals the committed file byte-for-byte (modulo line endings)', () => { - const generated = buildMatrix(registry).replace(/\r\r?\n/g, '\n').replace(/\r?\n+$/, '\n'); - const committed = fs.readFileSync(MATRIX, 'utf8').replace(/\r\r?\n/g, '\n').replace(/\r?\n+$/, '\n'); - assert.equal(committed, generated); - }); - +describe('capability-matrix content invariants (ADR-1244 Phase 6)', () => { test('every first-party capability in the registry appears as a matrix row', () => { const md = fs.readFileSync(MATRIX, 'utf8'); for (const cap of Object.values(registry.capabilities)) { diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index a994b2f87..cf9c3aa45 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -836,23 +836,6 @@ describe('normalizeLineEndings', () => { }); }); -describe('committed gsd-core/bin/lib/capability-registry.cjs is not stale', () => { - test('gen-capability-registry.cjs --check exits 0 (committed registry is up to date)', () => { - const result = spawnSync( - process.execPath, - [require('node:path').join(ROOT, 'scripts', 'gen-capability-registry.cjs'), '--check'], - { cwd: ROOT, encoding: 'utf8' }, - ); - assert.strictEqual( - result.status, - 0, - 'gen-capability-registry.cjs --check failed — committed capability-registry.cjs is stale.\n' + - 'Run: node scripts/gen-capability-registry.cjs --write\n' + - 'stderr: ' + (result.stderr || ''), - ); - }); -}); - // ─── 5. Registry shape from multiple capabilities ──────────────────────────── describe('registry structure', () => { diff --git a/tests/issue-498-package-identity.test.cjs b/tests/issue-498-package-identity.test.cjs index 4f23c9e8b..12293df14 100644 --- a/tests/issue-498-package-identity.test.cjs +++ b/tests/issue-498-package-identity.test.cjs @@ -15,7 +15,7 @@ const path = require('node:path'); const fs = require('node:fs'); const ROOT = path.join(__dirname, '..'); -const { deriveIdentity, formatManualInstall, render, slugifyPackageName } = require( +const { deriveIdentity, formatManualInstall, slugifyPackageName } = require( path.join(ROOT, 'scripts', 'generate-package-identity.cjs'), ); const GENERATED = path.join(ROOT, 'gsd-core', 'bin', 'lib', 'package-identity.cjs'); @@ -105,19 +105,11 @@ describe('Issue #498: formatManualInstall (the npx fallback command)', () => { }); }); -describe('Issue #498: generated runtime module (baked, drift-checked)', () => { - test('the committed generated file is in sync with package.json (no drift)', () => { - // Normalize line endings: on Windows the file is checked out with CRLF - // (no .gitattributes eol rule), while render() emits LF. The repo's - // convention is to compare normalized content (see autonomous-decomposition, - // bug-3707). The sync check is about content, not the checkout's eol. - const norm = (s) => s.replace(/\r\n/g, '\n'); - const expected = render(deriveIdentity(require(path.join(ROOT, 'package.json')))); - // allow-test-rule: architectural-invariant - const actual = fs.readFileSync(GENERATED, 'utf8'); - assert.equal(norm(actual), norm(expected), - 'package-identity.cjs is stale — run `node scripts/generate-package-identity.cjs`'); - }); +describe('Issue #498: generated runtime module (baked)', () => { + // The committed-generated-file freshness guard moved to + // `npm run lint:generated-sync` (generate-package-identity.cjs --check), where + // it runs against the committed file in both local and CI lint lanes instead + // of being masked by gsd-test's `npm run build` leg regenerating the artifact. test('requiring the generated module exposes the real coordinates', () => { const id = require(GENERATED); diff --git a/tests/issue-844-manifest-version-sync.test.cjs b/tests/issue-844-manifest-version-sync.test.cjs index 60cbcc8d1..ca8dd2b3a 100644 --- a/tests/issue-844-manifest-version-sync.test.cjs +++ b/tests/issue-844-manifest-version-sync.test.cjs @@ -338,17 +338,9 @@ describe('E: stageManifests — non-git dir is a no-op, not a throw', () => { }); // ─── D: CLI --check exits 0 when in sync ───────────────────────────────────── -describe('D: CLI --check exits 0 when manifests are in sync', () => { - - test('node scripts/sync-manifest-versions.cjs --check exits 0', () => { - // Will throw if exit code != 0 - execFileSync( - process.execPath, - [path.join(ROOT, 'scripts', 'sync-manifest-versions.cjs'), '--check'], - { cwd: ROOT } - ); - }); -}); +// Moved to `npm run lint:generated-sync` (sync-manifest-versions.cjs --check), +// which runs against the committed manifests in both local and CI lint lanes +// instead of being masked by gsd-test's `npm run build` leg. // ─── F: version script includes capability-registry regen (#1498) ───────────── // diff --git a/tests/loop-host-contract.test.cjs b/tests/loop-host-contract.test.cjs index 945b47eb3..d5d84ffbf 100644 --- a/tests/loop-host-contract.test.cjs +++ b/tests/loop-host-contract.test.cjs @@ -388,30 +388,17 @@ describe('buildContract cross-check drift guard', () => { // ─── 6. --check: CRLF-agnostic + committed-file staleness guard ────────────── -describe('normalizeLineEndings and committed-file staleness', () => { +describe('normalizeLineEndings', () => { test('normalizeLineEndings strips CR characters', () => { const crlf = 'line1\r\nline2\r\nline3'; const lf = 'line1\nline2\nline3'; assert.strictEqual(normalizeLineEndings(crlf), lf); assert.strictEqual(normalizeLineEndings(lf), lf); }); - - test('committed loop-host-contract.cjs is up to date (--check passes)', () => { - // Build the live contract from the real workflows - const contract = buildContract(); - const live = serializeContract(contract); - - // Read the committed file - const committed = fs.readFileSync(CONTRACT_PATH, 'utf8'); - - // FIX 4: Full-content comparison — no generated-by-line stripping needed - // because the serializer has no nondeterministic content (no timestamp). - assert.strictEqual( - normalizeLineEndings(committed), - normalizeLineEndings(live), - 'committed loop-host-contract.cjs is stale — run: node scripts/gen-loop-host-contract.cjs --write', - ); - }); + // The committed-loop-host-contract.cjs freshness guard moved to + // `npm run lint:generated-sync` (gen-loop-host-contract.cjs --check), where it + // runs against the committed file in both local and CI lint lanes — instead of + // being masked by gsd-test's `npm run build` leg regenerating the artifact. }); // ─── 7. STEP_WORKFLOWS and CANONICAL_POINTS exported constants ────────────────