Files
msd-core/tests/issue-844-manifest-version-sync.test.cjs
Tom Boucher 5fa4dcd78c fix: recover silently-excluded test dirs + test-architecture audit hardening (#1195)
* fix: recurse test discovery so subdir test suites actually run

scripts/run-tests.cjs discovered tests with a flat readdirSync(testDir),
silently excluding tests/observability/ (4 files), tests/dispatch/ (1) and
tests/installer-migrations/ (1) — 94 passing tests — from `npm test` and all
CI lanes. Walk the tree recursively (relative subpaths preserved), classify
suites by basename, and add a fail-on-zero-executed guard for suite/default
runs (escape hatch GSD_ALLOW_EMPTY_SUITE=1) while preserving the empty
--files/--files-from path the CI inert lane relies on.

Unit suite 735 -> 741 files; surfaces ADR-227's observability/dispatch seam.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: retire 5 verified-worthless tests

Adversarial verification confirmed these 5 prove nothing — their coverage is
provided more strictly elsewhere:
- enh-2790 'has a name: field' spot-checks (command-contract enforces /^gsd[:-]/)
- command-routing-hub duplicate construct + duplicate ERROR_KINDS assertions
- no-cjs-sdk-handsync-tooling (guarded files that never existed on main; bug-190
  covers the real retired SDK artifacts)
- runtime-artifact-layout cline edge case (subsumed by the explicit-global test
  and bug-782-cline-skills-emission)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: add ADR-218 release version-validation coverage

ADR-218 (reject leading-zero versions like 1.01.0; npm duplicate pre-check) had
zero tests — the logic lived only in release.yml bash. Add a test that extracts
the actual rejection regexes from the workflow and exercises them against a
boundary table (leading-zero/malformed rejected, valid accepted) plus structural
wiring assertions. Goes red if the regex is reverted to [0-9]+.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: redesign weak tests into behavioral, deterministic assertions

Per the ADR test audit, rewrite 27 weak test files (test-only, no source
changes) so each can go red for the defect it guards:
- kill pass-always assert.ok(true) placeholders (research-cli, worktree-baseref,
  bug-260 security guard, eslint-rules x24, clusters '|| true')
- replace source-text grep with behavioral calls (install Kilo, sh-hook-paths,
  plan-review-convergence) and add a repo-layout governance test
- de-flake real-clock/Math.random coupling (phase last_updated, bug-3707 mtime,
  context-utilization property, feat-3594)
- fix independence/shared-state violations (bug-492 singleton, issue-844 tmpRoot,
  core reapStaleTempFiles, active-workstream TTY, feat-488 GSD_HOME)
- strengthen property/shape-only tests (research-provider/store classification +
  collision) and unconditional plugin.json schema validation (issue-766)

Verified: all 28 files run together 1220 pass / 0 fail / 1 skip.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: add no-tautological-assert lint rule, error in test suite

New custom ESLint rule (eslint-rules/no-tautological-assert.cjs) bans asserts
that can never fail: assert(true)/assert.ok(<always-truthy literal>),
'cond || true' inside an assert, and equality asserts comparing two identical
literals. Wired as error on tests/**; full sweep confirmed zero existing
violations so the suite stays green. Prevents the placeholder-assert regressions
the audit redesigns just removed. RuleTester coverage added (6 valid, 8 invalid).

Note: no-only-tests was already enforced via eslint-plugin-no-only-tests, so no
duplicate rule was added.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: gate new allow-test-rule exemptions to require an issue ref

ADR-456 requires any allow-test-rule exemption added after the ADR to carry a
tracking issue number, but nothing enforced it. New ratchet gate
(scripts/lint-allow-test-rule-refs.cjs, wired into lint:ci) fails when a NEW
allow-test-rule comment lacks a #NNN/URL reference; the 323 existing untracked
exemptions are grandfathered in an allowlist that ratchets down as they gain
refs. Red-green verified (novel untracked offender fails; compliant passes).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs: add ADR test-audit evidence report (#1192)

Full risk-first qa-test-architect audit of the ADR portfolio (37 ADRs + 4
platform lenses, adversarial verification of retire verdicts) that drove the
P0 discovery fix, ADR-218 coverage, 5 retires, 27 redesigns, and the two new
lint gates. Filed as point-in-time evidence under docs/issueevidence/, named
for tracking issue #1192.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: replace pre-existing raw NUL byte with escape in feat-3594 fixture

feat-3594's null-byte parser fixture contained a literal NUL byte (pre-existing
on next at b10e5681 — confirmed: base blob has 1 NUL, this fix has 0), which
made git treat the file as binary and would break grep/editors. Switch to the
\x00 escape; the runtime string value (a real NUL in the parser input) is
unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: address adversarial-review findings

Codex adversarial pass over the branch:
- capability-registry drift test no longer mutates the committed generated
  capability-registry.cjs in place (concurrency hazard) — uses in-memory
  checkPipeline comparison instead.
- allow-test-rule ratchet now detects exemptions in ALL comment forms (block
  /* */ too, matching no-source-grep) so a block comment can't bypass it;
  one newly-surfaced pre-existing offender grandfathered (323->324).
- install.test Kilo case asserts on what install(false,'kilo') actually writes
  rather than manually calling configureKiloPermissions (masked the call site).
- issue-766 drops the undeclared transitive ajv dep for explicit structural
  assertions from the schema fixture.
- adr-218 test notes the hotfix leading-zero gap is tracked in #1186.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: address code-review findings (subdir discovery, rule + test gaps)

xhigh code review surfaced 15 confirmed issues, all fixed:
- run-tests.cjs --files now resolves subdir tests by bare basename + handles
  Windows backslash paths (ambiguous basenames error clearly).
- affected-tests-lib.cjs listTestFiles made recursive — the targeted CI lane was
  silently dropping changed subdir tests (same false-green class the audit fixed).
- no-tautological-assert now catches 'true || cond' and empty []/{}  equality.
- verify-test-quality: restore provenance-classification coverage, tighten the
  writeFile circular-detection check, guard the module-level file read.
- sh-hook-paths: cover the global-install .sh delegation branch (#2045 guard).
- active-workstream null-guard runs deterministically (no longer skipped on TTY).
- adr-218 structural guards tightened (major/minor leading-zero; needs: membership).
- repo-layout AGENTS.md guard no longer false-alarms on equivalent refactors.
- cross-ai ordering guard fails red when the step is missing.
- issue-766 parses required fields from the schema fixture (auto-enforced).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: stub USERPROFILE alongside HOME in feat-488 (Windows parity)

The feat-488 redesign stubbed process.env.HOME but not USERPROFILE; os.homedir()
resolves from USERPROFILE on Windows, so the home stub was not hermetic there —
caught by windows-test-parity-guard (stubsHomeNoUserProfile). Save/set/restore
USERPROFILE symmetrically with HOME (delete-if-originally-undefined).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: reconcile allow-test-rule allowlist after rebase onto next

Rebasing onto current next pulled in merged PR #1170, which added
inventory-headings-countfree.test.cjs (a baseline allow-test-rule exemption) and
deleted inventory-counts.test.cjs. Grandfather the former and prune the latter so
the ratchet matches the merged tree. No new debt from this PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-13 23:35:08 -04:00

263 lines
10 KiB
JavaScript

'use strict';
/**
* Regression tests for issue #844: manifest version sync.
*
* Verifies that `scripts/sync-manifest-versions.cjs` correctly stamps the
* package.json version into every registered runtime-integration manifest,
* and that all currently-tracked manifests are in sync.
*
* Key deliverable: the regression guard (test d) asserts that any committed
* JSON file with a top-level `version` field matching package.json is
* registered in VERSIONED_MANIFESTS — forcing explicit opt-in for future
* manifests.
*/
const { test, describe, before, after } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { execFileSync } = require('child_process');
const ROOT = path.resolve(__dirname, '..');
const helpers = require(path.join(__dirname, 'helpers.cjs'));
const {
VERSIONED_MANIFESTS,
syncManifestVersions,
getPackageVersion,
stageManifests,
} = require(path.join(ROOT, 'scripts', 'sync-manifest-versions.cjs'));
// ─── A: RED→GREEN repro via temp fixture ─────────────────────────────────────
//
// Each test in this describe operates on a single per-describe tmpRoot that is
// created in before() and torn down in after(). There are no setup/cleanup
// test() nodes — order-independence is guaranteed by the lifecycle hooks.
describe('A: syncManifestVersions — temp fixture', () => {
let tmpRoot;
before(() => {
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-'));
// Write tmp package.json
fs.writeFileSync(
path.join(tmpRoot, 'package.json'),
JSON.stringify({ name: 'x', version: '9.9.9-test.0' }, null, 2) + '\n'
);
// Copy real manifests into tmp, stamped at OLD version so tests can
// verify the pre-sync (stale) state and post-sync (updated) state.
for (const rel of VERSIONED_MANIFESTS) {
const realAbs = path.join(ROOT, rel);
const manifest = JSON.parse(fs.readFileSync(realAbs, 'utf8'));
manifest.version = '0.0.0';
const destAbs = path.join(tmpRoot, rel);
const destDir = path.dirname(destAbs);
if (!fs.existsSync(destDir)) fs.mkdirSync(destDir, { recursive: true });
fs.writeFileSync(destAbs, JSON.stringify(manifest, null, 2) + '\n');
}
// Run the sync once so post-sync assertions are valid.
syncManifestVersions({ root: tmpRoot });
});
after(() => {
helpers.cleanup(tmpRoot);
tmpRoot = null;
});
test('pre-sync fixture had at least one stale manifest (version 0.0.0 != 9.9.9-test.0)', () => {
// The before() hook wrote 0.0.0 into every manifest before syncing.
// We verify the sync actually had work to do by checking that any manifest
// that now reads 9.9.9-test.0 was not already at that version (0.0.0 ≠ 9.9.9-test.0).
// The simplest red-check: the fixture started with 0.0.0, which != 9.9.9-test.0.
assert.ok(
VERSIONED_MANIFESTS.length > 0,
'VERSIONED_MANIFESTS must be non-empty for the fixture to be meaningful'
);
// Independently confirm the target version != the stale seed
assert.notEqual('0.0.0', '9.9.9-test.0',
'Stale seed 0.0.0 must differ from fixture package.json version 9.9.9-test.0');
});
test('syncManifestVersions stamps all manifests to package.json version', () => {
const pkgVersion = getPackageVersion(tmpRoot);
assert.equal(pkgVersion, '9.9.9-test.0');
for (const rel of VERSIONED_MANIFESTS) {
const abs = path.join(tmpRoot, rel);
const m = JSON.parse(fs.readFileSync(abs, 'utf8'));
assert.equal(
m.version,
'9.9.9-test.0',
`${rel} version should be 9.9.9-test.0 after sync`
);
}
});
test('syncManifestVersions reports at least one changed file on first run', () => {
// Run a fresh sync against a freshly-stale fixture to observe the changed list.
// Create a separate sub-fixture so this test does not rely on before()'s sync order.
const sub = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-chk-'));
try {
fs.writeFileSync(
path.join(sub, 'package.json'),
JSON.stringify({ name: 'x', version: '9.9.9-test.0' }, null, 2) + '\n'
);
for (const rel of VERSIONED_MANIFESTS) {
const manifest = JSON.parse(fs.readFileSync(path.join(ROOT, rel), 'utf8'));
manifest.version = '0.0.0';
const destAbs = path.join(sub, rel);
const destDir = path.dirname(destAbs);
if (!fs.existsSync(destDir)) fs.mkdirSync(destDir, { recursive: true });
fs.writeFileSync(destAbs, JSON.stringify(manifest, null, 2) + '\n');
}
const changed = syncManifestVersions({ root: sub });
assert.ok(changed.length > 0, 'syncManifestVersions should report at least one changed file');
} finally {
helpers.cleanup(sub);
}
});
test('non-version fields are preserved after sync', () => {
for (const rel of VERSIONED_MANIFESTS) {
const real = JSON.parse(fs.readFileSync(path.join(ROOT, rel), 'utf8'));
const tmp = JSON.parse(fs.readFileSync(path.join(tmpRoot, rel), 'utf8'));
// Check that every non-version key from the real manifest exists in tmp
for (const key of Object.keys(real)) {
if (key === 'version') continue;
assert.ok(
Object.prototype.hasOwnProperty.call(tmp, key),
`${rel}: field "${key}" should be preserved after sync`
);
}
}
});
test('each synced file ends with a single trailing newline', () => {
for (const rel of VERSIONED_MANIFESTS) {
const raw = fs.readFileSync(path.join(tmpRoot, rel), 'utf8');
assert.ok(raw.endsWith('\n'), `${rel} must end with a trailing newline`);
assert.ok(!raw.endsWith('\n\n'), `${rel} must not end with a double newline`);
}
});
test('second syncManifestVersions call is idempotent (returns [])', () => {
// The before() already ran one sync. A second call must return [].
const changed = syncManifestVersions({ root: tmpRoot });
assert.deepEqual(changed, [], 'Second sync call should return [] (already in sync)');
});
});
// ─── B: Registry-in-sync: real manifests match package.json ──────────────────
describe('B: real manifests match package.json version', () => {
const pkgVersion = getPackageVersion(ROOT);
for (const rel of VERSIONED_MANIFESTS) {
test(`${rel} version === ${pkgVersion}`, () => {
const abs = path.join(ROOT, rel);
assert.ok(fs.existsSync(abs), `${rel} must exist at ${abs}`);
const m = JSON.parse(fs.readFileSync(abs, 'utf8'));
assert.equal(
m.version,
pkgVersion,
`${rel} version (${m.version}) must match package.json version (${pkgVersion}). ` +
'Run `node scripts/sync-manifest-versions.cjs` to fix.'
);
});
}
});
// ─── C: Regression guard — all version-bearing JSON files are registered ──────
describe('C: regression guard — version-bearing JSON files must be registered', () => {
// package.json is the version source; package-lock.json is npm-managed.
// Both inherently track the version without the sync script.
const ALLOWED = new Set([...VERSIONED_MANIFESTS, 'package.json', 'package-lock.json']);
// Semver-ish: matches X.Y.Z with optional pre-release/build metadata.
const SEMVER = /^\d+\.\d+\.\d+(?:[-+].+)?$/;
// Paths to exclude from the guard
const EXCLUDED_PREFIXES = ['tests/', 'node_modules/', '.changeset/', 'docs/'];
test('every committed JSON with a semver top-level version is registered or explicitly allowed', (t) => {
// Enumerate committed JSON files via git (no pathspec to avoid recursion quirks;
// filter to .json in JS instead).
let lines;
try {
const out = execFileSync('git', ['ls-files'], { cwd: ROOT });
lines = out.toString().split('\n').filter((f) => f.endsWith('.json'));
} catch (err) {
t.skip('git unavailable: ' + err.message);
return;
}
for (const rel of lines) {
if (ALLOWED.has(rel)) continue;
if (EXCLUDED_PREFIXES.some(prefix => rel.startsWith(prefix))) continue;
const abs = path.join(ROOT, rel);
let parsed;
try {
parsed = JSON.parse(fs.readFileSync(abs, 'utf8'));
} catch (_) {
continue; // skip invalid JSON (shouldn't exist, but be safe)
}
if (
parsed &&
typeof parsed === 'object' &&
!Array.isArray(parsed) &&
typeof parsed.version === 'string' &&
SEMVER.test(parsed.version)
) {
assert.ok(
ALLOWED.has(rel),
`${rel} has a semver top-level "version" but is not registered in ` +
'scripts/sync-manifest-versions.cjs VERSIONED_MANIFESTS (nor an npm-managed file). ' +
"Register it so 'npm version' keeps it in sync (issue #844)."
);
}
}
});
});
// ─── E: stageManifests in a non-git dir must not throw ───────────────────────
describe('E: stageManifests — non-git dir is a no-op, not a throw', () => {
test('stageManifests({root}) with a non-git tempdir warns and returns without throwing', () => {
const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-nogit-'));
try {
// Write a minimal package.json so getPackageVersion doesn't error if called
fs.writeFileSync(
path.join(tmpRoot, 'package.json'),
JSON.stringify({ name: 'x', version: '0.0.0' }, null, 2) + '\n'
);
// Must not throw even though tmpRoot is not a git repo
assert.doesNotThrow(() => {
stageManifests({ root: tmpRoot });
}, 'stageManifests must not throw outside a git work tree');
} finally {
helpers.cleanup(tmpRoot);
}
});
});
// ─── 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 }
);
});
});