diff --git a/tests/emitted-attribution.test.cjs b/tests/emitted-attribution.test.cjs index 6f486ba3e..e98709f0d 100644 --- a/tests/emitted-attribution.test.cjs +++ b/tests/emitted-attribution.test.cjs @@ -36,7 +36,7 @@ const path = require('node:path'); const { execFileSync } = require('node:child_process'); const fc = require('fast-check'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, createTempDir } = require('./helpers.cjs'); const { BUILD_SCRIPT } = require('./helpers/install-shared.cjs'); const { resolveChangedPaths, @@ -47,9 +47,16 @@ const { currentManifests, currentSizes, readAckFile, + baselineFamilyNamesAtRef, + MANIFEST_FAMILIES, + MINIMUM_MANIFEST_FAMILIES, + REGISTRY_SIGNAL_PATHS, + FAMILY_REASON, + touchesRuntimeRegistry, + reconcileFamilies, } = require('./helpers/emitted-runtime.cjs'); -const { EXPECTED_MANIFEST_COUNT } = require('./helpers/emitted-provenance.cjs'); +const { EXPECTED_MANIFEST_COUNT, loadManifests } = require('./helpers/emitted-provenance.cjs'); const { ACK_VERSION, sourceSatisfiedBy, @@ -656,6 +663,416 @@ test('property: every moved key lands in exactly one bucket', () => { ); }); +// ─── Family reconciliation (#2723 correction) ──────────────────────────────── +// +// #2723 shipped `EXPECTED_MANIFEST_COUNT = 19` asserted against BOTH the baseline (built +// at the base ref) and the current tree (built at PR HEAD). Those sides legitimately +// differ by one family whenever a PR adds or removes a runtime, so no value satisfied +// both: 19 rejected the current side, 20 rejected the baseline side. Every runtime-adding +// PR was hard-blocked — found by tracing #2005 (Qoder) through the gate. +// +// Driven at PURE-FUNCTION altitude on purpose. The real-tree test below skips wherever no +// base ref exists (the gsd-test runner shallow-clones, so `origin/*` is absent), so a +// regression written at that altitude would silently skip on the very runner that has to +// prove RED. + +const ALL_FAMILIES = MANIFEST_FAMILIES.map((f) => f.name); +const REGISTRY_CHANGE = ['tests/helpers/install-shared.cjs']; +// The shape a shipping caller passes: repo-relative POSIX paths from `git diff --name-only`. +const CONTENT_ONLY_CHANGE = ['gsd-core/workflows/plan-phase.md']; + +const derivedOf = (names) => names.map((name) => ({ name, runtime: name, scope: 'global' })); +const manifestsOf = (names) => Object.fromEntries(names.map((n) => [n, { 'some/emitted/path': 'hash' }])); + +/** Build a fully-consistent reconciliation input, then override one facet per test. */ +function reconcileWith({ derivedNames = ALL_FAMILIES, fixtureNames, baselineNames, currentNames, ...rest }) { + return reconcileFamilies({ + derived: derivedOf(derivedNames), + fixtures: fixtureNames || derivedNames, + baseline: manifestsOf(baselineNames || derivedNames), + current: manifestsOf(currentNames || derivedNames), + changedPaths: CONTENT_ONLY_CHANGE, + ...rest, + }); +} + +const codesOf = (r) => r.errors.map((e) => e.code); + +test('reason codes are a frozen, locked set', () => { + assert.deepEqual(Object.keys(FAMILY_REASON).sort(), [ + 'ADDED_UNATTRIBUTED', 'BAD_CHANGED_PATHS', 'BASELINE_UNUSABLE', 'BELOW_FLOOR', + 'CURRENT_UNUSABLE', 'DERIVED_UNUSABLE', 'DROPPED_UNATTRIBUTED', + 'FIXTURES_UNUSABLE', 'FIXTURE_WITHOUT_RUNTIME', 'MISSING_CLAUDE_LOCAL', + 'RUNTIME_WITHOUT_FIXTURE', + ]); + assert.ok(Object.isFrozen(FAMILY_REASON)); +}); + +test('passes when every family signal agrees', () => { + assert.deepEqual(reconcileWith({}), { ok: true, errors: [] }); +}); + +test('the count export agrees with the derived family set (divergence guard)', () => { + // The #2723 defect was two surfaces carrying independent notions of this number. + assert.equal(EXPECTED_MANIFEST_COUNT, MANIFEST_FAMILIES.length); + assert.ok(EXPECTED_MANIFEST_COUNT >= MINIMUM_MANIFEST_FAMILIES); +}); + +// ── The deadlock itself ────────────────────────────────────────────────────── + +test('permits an added family attributed to a runtime-registry change', () => { + const withQoder = [...ALL_FAMILIES, 'qoder']; + const r = reconcileWith({ + derivedNames: withQoder, + baselineNames: ALL_FAMILIES, // base ref predates the new runtime + currentNames: withQoder, + changedPaths: REGISTRY_CHANGE, + }); + assert.deepEqual(r, { ok: true, errors: [] }); +}); + +test('rejects an added family with no runtime-registry change, naming it', () => { + const withQoder = [...ALL_FAMILIES, 'qoder']; + const r = reconcileWith({ + derivedNames: withQoder, + baselineNames: ALL_FAMILIES, + currentNames: withQoder, + changedPaths: CONTENT_ONLY_CHANGE, + }); + assert.equal(r.ok, false); + assert.deepEqual(r.errors, [{ code: FAMILY_REASON.ADDED_UNATTRIBUTED, family: 'qoder' }]); +}); + +test('permits a dropped family attributed to a runtime-registry change', () => { + const without = ALL_FAMILIES.filter((n) => n !== 'trae'); + const r = reconcileWith({ + derivedNames: without, + baselineNames: ALL_FAMILIES, + currentNames: without, + changedPaths: REGISTRY_CHANGE, + minimum: 18, + }); + assert.deepEqual(r, { ok: true, errors: [] }); +}); + +test('rejects a silently dropped family, naming it', () => { + const without = ALL_FAMILIES.filter((n) => n !== 'trae'); + const r = reconcileWith({ + derivedNames: without, + baselineNames: ALL_FAMILIES, + currentNames: without, + changedPaths: CONTENT_ONLY_CHANGE, + minimum: 18, + }); + assert.equal(r.ok, false); + assert.deepEqual(r.errors, [{ code: FAMILY_REASON.DROPPED_UNATTRIBUTED, family: 'trae' }]); +}); + +test('attribution is the ONLY permission path, symmetrically', () => { + // No ack-style bypass on either side: a one-sided escape hatch would make removals + // easier to wave through than additions, and the drift-ack file covers unattributable + // emitted-PATH deltas, not family churn. + const without = ALL_FAMILIES.filter((n) => n !== 'trae'); + const added = [...ALL_FAMILIES, 'qoder']; + for (const [names, baselineNames, code, family] of [ + [without, ALL_FAMILIES, FAMILY_REASON.DROPPED_UNATTRIBUTED, 'trae'], + [added, ALL_FAMILIES, FAMILY_REASON.ADDED_UNATTRIBUTED, 'qoder'], + ]) { + const r = reconcileWith({ + derivedNames: names, baselineNames, currentNames: names, + changedPaths: CONTENT_ONLY_CHANGE, minimum: 18, + }); + assert.equal(r.ok, false); + assert.deepEqual(r.errors, [{ code, family }]); + } +}); + +test('an add and a drop together are permitted when attributed', () => { + const swapped = [...ALL_FAMILIES.filter((n) => n !== 'trae'), 'qoder']; + const r = reconcileWith({ + derivedNames: swapped, + baselineNames: ALL_FAMILIES, + currentNames: swapped, + changedPaths: REGISTRY_CHANGE, + }); + assert.deepEqual(r, { ok: true, errors: [] }); +}); + +test('an EQUAL-COUNT membership swap is caught in both directions', () => { + // 19 in, 19 out — invisible to any count-based check. This is why the contract is + // set-based rather than numeric. + const swapped = [...ALL_FAMILIES.filter((n) => n !== 'trae'), 'qoder']; + const r = reconcileWith({ + derivedNames: swapped, + baselineNames: ALL_FAMILIES, + currentNames: swapped, + changedPaths: CONTENT_ONLY_CHANGE, + }); + assert.equal(swapped.length, ALL_FAMILIES.length, 'the swap must leave the totals equal'); + assert.equal(r.ok, false); + assert.deepEqual(r.errors.slice().sort((a, b) => a.code.localeCompare(b.code)), [ + { code: FAMILY_REASON.ADDED_UNATTRIBUTED, family: 'qoder' }, + { code: FAMILY_REASON.DROPPED_UNATTRIBUTED, family: 'trae' }, + ]); +}); + +// ── Single-tree drift ──────────────────────────────────────────────────────── + +test('rejects a fixture with no registered runtime, naming it', () => { + const r = reconcileWith({ fixtureNames: [...ALL_FAMILIES, 'ghost'] }); + assert.equal(r.ok, false); + assert.deepEqual(r.errors, [{ code: FAMILY_REASON.FIXTURE_WITHOUT_RUNTIME, family: 'ghost' }]); +}); + +test('rejects a registered runtime with no fixture, naming it', () => { + const r = reconcileWith({ + derivedNames: [...ALL_FAMILIES, 'qoder'], + fixtureNames: ALL_FAMILIES, + baselineNames: [...ALL_FAMILIES, 'qoder'], + currentNames: [...ALL_FAMILIES, 'qoder'], + changedPaths: REGISTRY_CHANGE, + }); + assert.equal(r.ok, false); + assert.deepEqual(r.errors, [{ code: FAMILY_REASON.RUNTIME_WITHOUT_FIXTURE, family: 'qoder' }]); +}); + +// ── The absolute floor: limit-1 / limit / limit+1 ──────────────────────────── + +test('floor is enforced at limit-1 / limit / limit+1', () => { + const eighteen = ALL_FAMILIES.filter((n) => n !== 'trae'); // limit-1 + const twenty = [...ALL_FAMILIES, 'qoder']; // limit+1 + + const below = reconcileWith({ + derivedNames: eighteen, baselineNames: eighteen, currentNames: eighteen, + }); + assert.equal(below.ok, false); + assert.ok(codesOf(below).includes(FAMILY_REASON.BELOW_FLOOR)); + + assert.deepEqual(reconcileWith({}), { ok: true, errors: [] }); // limit == 19 + + const above = reconcileWith({ + derivedNames: twenty, baselineNames: twenty, currentNames: twenty, + }); + assert.deepEqual(above, { ok: true, errors: [] }); +}); + +test('a uniformly shrunken universe fails on the floor', () => { + // The Goodhart move the old literal permitted: drop a runtime AND its fixture together + // and lower the constant, and 18 === 18 passes over a smaller world. + const eighteen = ALL_FAMILIES.filter((n) => n !== 'trae'); + const r = reconcileWith({ + derivedNames: eighteen, fixtureNames: eighteen, + baselineNames: eighteen, currentNames: eighteen, + changedPaths: REGISTRY_CHANGE, + }); + assert.equal(r.ok, false); + assert.deepEqual(codesOf(r), [FAMILY_REASON.BELOW_FLOOR]); +}); + +// ── #2086: claude-local is pinned by name on both sides ────────────────────── + +test('a missing claude-local family is named on either side', () => { + const noLocal = ALL_FAMILIES.filter((n) => n !== 'claude-local'); + const missingCurrent = reconcileWith({ + currentNames: noLocal, changedPaths: REGISTRY_CHANGE, + }); + assert.ok(codesOf(missingCurrent).includes(FAMILY_REASON.MISSING_CLAUDE_LOCAL)); + + const missingBaseline = reconcileWith({ + baselineNames: noLocal, changedPaths: REGISTRY_CHANGE, + }); + assert.ok(codesOf(missingBaseline).includes(FAMILY_REASON.MISSING_CLAUDE_LOCAL)); +}); + +// ── Hostile / malformed input: explicit failure, never a quiet ok ──────────── + +test('unusable baseline and current are rejected explicitly, not read as empty', () => { + for (const bad of [null, undefined, [], 'nope', 0]) { + const r = reconcileFamilies({ + derived: derivedOf(ALL_FAMILIES), fixtures: ALL_FAMILIES, + baseline: bad, current: manifestsOf(ALL_FAMILIES), changedPaths: [], + }); + assert.deepEqual(r, { ok: false, errors: [{ code: FAMILY_REASON.BASELINE_UNUSABLE }] }); + } + for (const bad of [null, undefined, [], 'nope', 0]) { + const r = reconcileFamilies({ + derived: derivedOf(ALL_FAMILIES), fixtures: ALL_FAMILIES, + baseline: manifestsOf(ALL_FAMILIES), current: bad, changedPaths: [], + }); + assert.deepEqual(r, { ok: false, errors: [{ code: FAMILY_REASON.CURRENT_UNUSABLE }] }); + } +}); + +test('a non-array changedPaths is an explicit error, never a silent "no registry change"', () => { + for (const bad of [null, undefined, 'tests/helpers/install-shared.cjs', {}, 7]) { + const r = reconcileWith({ changedPaths: bad }); + assert.deepEqual(r, { ok: false, errors: [{ code: FAMILY_REASON.BAD_CHANGED_PATHS }] }); + } +}); + +test('malformed derived and fixtures inputs fail with a verdict, not a TypeError', () => { + // Every input is gated. An unhandled throw here would read as an infrastructure fault + // rather than a gate verdict, which is how a propagation check goes quiet. + for (const bad of [null, undefined, 'nope', {}, [{ nope: 1 }], [null]]) { + const r = reconcileFamilies({ + derived: bad, fixtures: ALL_FAMILIES, + baseline: manifestsOf(ALL_FAMILIES), current: manifestsOf(ALL_FAMILIES), + changedPaths: [], + }); + assert.deepEqual(r, { ok: false, errors: [{ code: FAMILY_REASON.DERIVED_UNUSABLE }] }); + } + for (const bad of [null, undefined, 'nope', {}, [1], [null]]) { + const r = reconcileFamilies({ + derived: derivedOf(ALL_FAMILIES), fixtures: bad, + baseline: manifestsOf(ALL_FAMILIES), current: manifestsOf(ALL_FAMILIES), + changedPaths: [], + }); + assert.deepEqual(r, { ok: false, errors: [{ code: FAMILY_REASON.FIXTURES_UNUSABLE }] }); + } +}); + +// ── Registry attribution ───────────────────────────────────────────────────── + +test('each registry-signal path independently attributes a family change', () => { + for (const p of [...REGISTRY_SIGNAL_PATHS, 'capabilities/qoder/capability.json']) { + assert.equal(touchesRuntimeRegistry([p]), true, `${p} should attribute`); + } + // Narrow on purpose: surfaces that merely accompany a runtime addition must NOT + // excuse an unattributed family delta. + for (const p of ['src/runtime-name-policy.cts', 'gsd-core/bin/lib/capability-registry.cjs']) { + assert.equal(touchesRuntimeRegistry([p]), false, `${p} must NOT attribute on its own`); + } +}); + +test('backslash-separated registry paths normalize unconditionally', () => { + // Path separators normalize on every platform — backslash paths arrive on Linux too. + assert.equal(touchesRuntimeRegistry(['tests\\helpers\\install-shared.cjs']), true); + assert.equal(touchesRuntimeRegistry(['capabilities\\qoder\\capability.json']), true); +}); + +test('near-miss paths do not attribute a family change', () => { + for (const p of [ + 'capabilities/qoder/other.json', + 'capabilities/capability.json', + 'tests/helpers/install-shared.cjs.bak', + 'docs/tests/helpers/install-shared.cjs', + 'gsd-core/workflows/plan-phase.md', + ]) { + assert.equal(touchesRuntimeRegistry([p]), false, `${p} should NOT attribute`); + } + assert.equal(touchesRuntimeRegistry([]), false); +}); + +// ── The baseline must come from the REF, not from HEAD's registry ──────────── + +test('baseline families are enumerated from the ref, not from the current registry', (t) => { + // Regression: enumerating the baseline from MANIFEST_FAMILIES (imported at module load, + // so it describes PR HEAD) makes a REMOVED runtime invisible — the name is already gone + // from the current registry, so the base ref is never asked for it, and the dropped- + // family check can never fire in production even though its unit tests pass. + // + // Built as its own git repo rather than reaching for this repo's history. The gsd-test + // runner shallow-clones base+head, so `rev-list --max-parents=0` there returns the + // GRAFTED boundary commit — a recent one carrying every fixture — not a true root. (This + // repo also has two root commits locally.) A history-dependent assertion passes on a full + // clone and fails in the runner, which is exactly what it did. + const repo = createTempDir('emitted-baseline-ref'); + t.after(() => cleanup(repo)); + const run = (...args) => execFileSync('git', args, { cwd: repo, encoding: 'utf8', timeout: 30_000 }); + + run('init', '--quiet', '-b', 'main'); + run('config', 'user.email', 'test@example.invalid'); + run('config', 'user.name', 'Test'); + const fixtureDir = path.join(repo, ...'tests/fixtures/golden-install-parity'.split('/')); + fs.mkdirSync(fixtureDir, { recursive: true }); + + // Deliberately includes a family that is NOT in today's registry. This is the real + // discriminator: a registry-derived implementation can never report it, because the name + // does not exist in MANIFEST_FAMILIES — which is precisely how a REMOVED runtime went + // invisible and made the dropped-family check unreachable in production. + const atRefOnly = 'zzz-retired-runtime'; + const committed = ['claude', 'claude-local', atRefOnly]; + for (const name of committed) { + fs.writeFileSync(path.join(fixtureDir, `${name}.json`), JSON.stringify({ 'a/b': 'hash' })); + } + run('add', '-A'); + run('commit', '--quiet', '-m', 'fixtures'); + + assert.ok( + !ALL_FAMILIES.includes(atRefOnly), + 'the probe family must be absent from the current registry for this test to discriminate', + ); + assert.deepEqual( + baselineFamilyNamesAtRef('HEAD', { cwd: repo }).slice().sort(), + committed.slice().sort(), + 'the baseline must report what the REF carries, including a family the current registry lacks', + ); + + // A ref that cannot be resolved yields nothing rather than throwing, which is the + // post-cutover signal to fall back to resolveBaseline's cache path. + assert.deepEqual(baselineFamilyNamesAtRef('refs/heads/no-such-ref-2723', { cwd: repo }), []); + + // Deliberately NOT asserted against the ambient checkout. Reading this repo's own HEAD is + // not guaranteed inside the runner container — it returned [] there, which is this + // function's documented behavior when git cannot read the ref, not a defect. Asserting on + // it tests the checkout rather than the code, and the temp repo above already proves the + // property that matters: the family set follows the REF. The ambient path is covered by + // the real-tree test, which skips explicitly when no base ref is resolvable. + // + // A git failure is never silently permissive downstream: baselineManifestsAtRef returns + // null on an empty family set, and the real-tree test asserts the baseline is non-empty. +}); + +// ── Independence / purity ──────────────────────────────────────────────────── + +test('reconciliation is pure across repeated calls', () => { + const args = { + derivedNames: [...ALL_FAMILIES, 'qoder'], + baselineNames: ALL_FAMILIES, + currentNames: [...ALL_FAMILIES, 'qoder'], + changedPaths: CONTENT_ONLY_CHANGE, + }; + assert.deepEqual(reconcileWith(args), reconcileWith(args)); +}); + +// ── Property: the reported delta is exactly the set difference ─────────────── + +test('property: reported added/dropped are exactly the set differences', () => { + fc.assert( + fc.property( + fc.uniqueArray(fc.string({ minLength: 1, maxLength: 6 }).filter((s) => !/^\s*$/.test(s)), { minLength: 0, maxLength: 5 }), + fc.uniqueArray(fc.integer({ min: 0, max: ALL_FAMILIES.length - 2 }), { minLength: 0, maxLength: 4 }), + (rawAdds, dropIdx) => { + const added = rawAdds.filter((s) => !ALL_FAMILIES.includes(s)); + // never drop claude-local: it has its own dedicated assertion + const dropped = dropIdx + .map((i) => ALL_FAMILIES[i]) + .filter((n) => n !== 'claude-local'); + const current = [...ALL_FAMILIES.filter((n) => !dropped.includes(n)), ...added]; + + const r = reconcileFamilies({ + derived: derivedOf(current), + fixtures: current, + baseline: manifestsOf(ALL_FAMILIES), + current: manifestsOf(current), + changedPaths: CONTENT_ONLY_CHANGE, + minimum: 0, + }); + + const reportedAdded = r.errors + .filter((e) => e.code === FAMILY_REASON.ADDED_UNATTRIBUTED).map((e) => e.family).sort(); + const reportedDropped = r.errors + .filter((e) => e.code === FAMILY_REASON.DROPPED_UNATTRIBUTED).map((e) => e.family).sort(); + + assert.deepEqual(reportedAdded, [...new Set(added)].sort()); + assert.deepEqual(reportedDropped, [...new Set(dropped)].sort()); + return true; + }, + ), + { numRuns: 200, seed: 2723 }, + ); +}); + // ─── The real thing: the law, run against the actual tree ─────────────────── // // Everything above exercises the pure law against synthetic input, which is what makes @@ -719,22 +1136,25 @@ test('differential attribution over the real tree', { timeout: 900_000 }, async const ack = readAckFile(); const current = currentManifests(); - // Assert against the INDEPENDENT expected count, not just baseline-vs-current. - // Comparing the two sides to each other cannot catch a family dropped from BOTH — - // which is exactly what happened with claude-local: 18 === 18 passed vacuously while - // the 19th family went unchecked. - assert.equal( - Object.keys(baseline).length, - EXPECTED_MANIFEST_COUNT, - `baseline must cover all ${EXPECTED_MANIFEST_COUNT} emitted manifest families`, + // Reconcile the family SET across three independent signals, rather than asserting one + // count against both sides. The baseline is built at the base ref and the current tree + // at PR HEAD, so the two legitimately differ by a family whenever a PR adds or removes + // a runtime — a single shared literal could satisfy neither side at once (#2723), and + // a count cannot see a membership swap that leaves the total unchanged either way. + const familyVerdict = reconcileFamilies({ + derived: MANIFEST_FAMILIES, + fixtures: loadManifests().map((m) => m.file.replace(/\.json$/, '')), + baseline, + current, + changedPaths, + }); + assert.ok( + familyVerdict.ok, + 'emitted manifest family set is not reconciled:\n ' + + familyVerdict.errors + .map((e) => (e.family ? `${e.code}: ${e.family}` : e.code)) + .join('\n '), ); - assert.equal( - Object.keys(current).length, - EXPECTED_MANIFEST_COUNT, - `current must cover all ${EXPECTED_MANIFEST_COUNT} emitted manifest families`, - ); - assert.ok(baseline['claude-local'], 'the claude local-scope layout (#2086) must be covered'); - assert.ok(current['claude-local'], 'the claude local-scope layout (#2086) must be covered'); const result = diffEmitted({ baseline, diff --git a/tests/helpers/emitted-provenance.cjs b/tests/helpers/emitted-provenance.cjs index 38c1fa9f4..a07e00971 100644 --- a/tests/helpers/emitted-provenance.cjs +++ b/tests/helpers/emitted-provenance.cjs @@ -44,9 +44,22 @@ const path = require('node:path'); const REPO_ROOT = path.join(__dirname, '..', '..'); const FIXTURES_DIR = path.join(REPO_ROOT, 'tests', 'fixtures', 'golden-install-parity'); -/** Number of runtime manifests the guard expects to cover. Asserted, so a glob that - * silently matches fewer files can never report a vacuous pass. */ -const EXPECTED_MANIFEST_COUNT = 19; +const { MANIFEST_FAMILIES } = require('./install-shared.cjs'); + +/** + * Number of runtime manifests the guard expects to cover. Asserted, so a glob that + * silently matches fewer files can never report a vacuous pass. + * + * DERIVED, not a literal (#2723). It was `19`, and that same literal was also asserted + * against the baseline built at the base ref — two trees that legitimately differ by one + * family whenever a PR adds or removes a runtime, which made every such PR unpassable at + * any value. Deriving it from the single `MANIFEST_FAMILIES` source keeps the + * anti-vacuity property here (this tree's glob must match this tree's registry) while + * leaving the cross-tree question to `reconcileFamilies`, which is set-based and + * direction-aware. The absolute floor that a shrunken universe cannot satisfy lives with + * the derivation as `MINIMUM_MANIFEST_FAMILIES`. + */ +const EXPECTED_MANIFEST_COUNT = MANIFEST_FAMILIES.length; // ─── Emitted roots ──────────────────────────────────────────────────────────── // Longest-first: `skills/gsd` (hermes category dir) must win over `skills` for diff --git a/tests/helpers/emitted-runtime.cjs b/tests/helpers/emitted-runtime.cjs index 6b007c49c..6c1c94ddb 100644 --- a/tests/helpers/emitted-runtime.cjs +++ b/tests/helpers/emitted-runtime.cjs @@ -36,7 +36,8 @@ const { execFileSync } = require('node:child_process'); const { cleanup } = require('../helpers.cjs'); const { - RUNTIME_META, + MANIFEST_FAMILIES, + MINIMUM_MANIFEST_FAMILIES, runMinimalInstall, buildParityManifest, } = require('./install-shared.cjs'); @@ -46,26 +47,181 @@ const ACK_PATH = path.join(REPO_ROOT, 'tests', 'emitted-drift-ack.json'); const FIXTURE_SUBDIR = 'tests/fixtures/golden-install-parity'; /** - * The emitted manifest families, as (fixtureName -> install spec). + * Repo paths whose presence in a PR diff attributes a CHANGE TO THE FAMILY SET — + * a runtime being added or removed — as opposed to a change in emitted content. * - * NOT simply `Object.keys(RUNTIME_META)`: that has 18 entries while the fixture set has - * 19. The extra one is `claude-local` — claude is the reference host and the ONLY - * runtime with a distinct LOCAL "legacy flat-commands" layout (`commands/gsd-*.md` + - * `agents/gsd-*.md` at project scope), which `golden-install-parity.test.cjs` guards - * with a hand-coded test outside its RUNTIME_META loop (#2086). + * Deliberately NARROW: only the two surfaces that actually define the family set — + * `RUNTIME_META`'s home, and a runtime's capability descriptor. Every extra path here + * widens what silently excuses an unattributed family delta, so adjacent surfaces that + * merely *accompany* a runtime addition (name-policy, capability registry) are left out + * on purpose. A PR that adds a runtime necessarily touches one of these two. * - * Enumerating from RUNTIME_META alone dropped that family from BOTH sides of the - * differential, so a same-count self-check (18 === 18) passed vacuously and a PR - * changing Claude's local-scope output would fail the golden while this check reported - * ok. That disagreement is exactly what the dual-run window is meant to surface as a - * provenance-table hole — so a wiring omission masquerading as one is the worst - * possible failure here. Derived explicitly, and asserted against the fixture count. + * Deliberately path-based rather than diff-hunk-parsing: asserting that a diff adds a + * specific `RUNTIME_META` key would be a source-grep test, which this repo prohibits. + * The residual — a PR touching one of these for an unrelated reason may permit an + * otherwise-unexplained family delta — is recorded in the ADR-2719 risk register and is + * one class weaker than the false-attribution risk already accepted there. */ -const MANIFEST_FAMILIES = [ - ...Object.keys(RUNTIME_META).map((runtime) => ({ name: runtime, runtime, scope: 'global' })), - { name: 'claude-local', runtime: 'claude', scope: 'local' }, +const REGISTRY_SIGNAL_PATHS = [ + 'tests/helpers/install-shared.cjs', ]; +/** + * A capability descriptor: `capabilities//capability.json`, exactly one segment deep. + * + * Anchored, with `[^/]+` for the runtime segment. A prefix+suffix pair is NOT equivalent and + * was wrong: `capabilities/capability.json` satisfies both `startsWith('capabilities/')` and + * `endsWith('/capability.json')` with no runtime segment at all, and + * `capabilities/a/b/capability.json` satisfies them at the wrong depth. Both would have + * excused an unattributed family delta. + */ +const REGISTRY_SIGNAL_PATTERN = /^capabilities\/[^/]+\/capability\.json$/; + +/** + * Reason codes for family reconciliation. + * + * Frozen and asserted as a set, so adding a code is a coordinated three-part change + * (enum, emitter, the test that locks the key list). Tests assert on these codes, never + * on rendered prose — the repo prohibits raw text matching on produced output. + */ +const FAMILY_REASON = Object.freeze({ + BELOW_FLOOR: 'below_floor', + FIXTURE_WITHOUT_RUNTIME: 'fixture_without_runtime', + RUNTIME_WITHOUT_FIXTURE: 'runtime_without_fixture', + ADDED_UNATTRIBUTED: 'added_unattributed', + DROPPED_UNATTRIBUTED: 'dropped_unattributed', + MISSING_CLAUDE_LOCAL: 'missing_claude_local', + BASELINE_UNUSABLE: 'baseline_unusable', + CURRENT_UNUSABLE: 'current_unusable', + DERIVED_UNUSABLE: 'derived_unusable', + FIXTURES_UNUSABLE: 'fixtures_unusable', + BAD_CHANGED_PATHS: 'bad_changed_paths', +}); + +/** Path separators normalize UNCONDITIONALLY — backslash paths arrive on Linux too. */ +function toPosix(p) { + return String(p).replace(/\\/g, '/'); +} + +/** True when `changedPaths` plausibly alters the runtime registry. */ +function touchesRuntimeRegistry(changedPaths) { + return changedPaths.some((raw) => { + const p = toPosix(raw); + return REGISTRY_SIGNAL_PATHS.includes(p) || REGISTRY_SIGNAL_PATTERN.test(p); + }); +} + +/** + * Reconcile the emitted manifest FAMILY SET across the three independent signals. + * + * ── Why this is not a count ────────────────────────────────────────────────── + * #2723 shipped a single literal (`EXPECTED_MANIFEST_COUNT = 19`) asserted against both + * the baseline (built at the base ref) and the current tree (built at PR HEAD). Those + * two legitimately differ by one family whenever a PR adds or removes a runtime, so no + * value of that literal could satisfy both: 19 rejected the current side, 20 rejected + * the baseline side. Every PR adding a runtime was hard-blocked. + * + * Equally important, a count cannot see a MEMBERSHIP SWAP — add one family and remove + * another and the totals still match while both changes go unexamined. The contract is + * therefore set-based in both directions. + * + * ── The three signals ──────────────────────────────────────────────────────── + * derived what the runtime registry says this tree emits (MANIFEST_FAMILIES) + * fixtures what this tree has recorded (the committed glob) + * baseline what existed before this PR (families at the base ref) + * + * derived-vs-fixtures catches drift on a single tree; baseline-vs-current catches an + * unexplained change to the set; and the floor catches the case neither can — a universe + * that shrank uniformly, which a same-count self-check passes vacuously. + * + * Pure and IO-free by construction: the real-tree caller skips wherever no base ref + * exists (the gsd-test runner shallow-clones, so `origin/*` is absent), which would make + * a regression written at that altitude silently skip instead of proving anything. + * + * @param {object} o + * @param {Array<{name:string}>} o.derived families the registry implies + * @param {string[]} o.fixtures family names recorded on this tree + * @param {object|null} o.baseline manifests at the base ref (keyed by family) + * @param {object|null} o.current manifests at PR HEAD (keyed by family) + * @param {string[]} o.changedPaths repo-relative paths this PR changed + * @param {number} [o.minimum] absolute floor + * @returns {{ok: boolean, errors: Array<{code: string, family?: string}>}} + */ +function reconcileFamilies({ + derived, + fixtures, + baseline, + current, + changedPaths, + minimum = MINIMUM_MANIFEST_FAMILIES, +} = {}) { + const errors = []; + const add = (code, family) => errors.push(family ? { code, family } : { code }); + + // Hostile-input gates first, and EVERY input gets one. Each returns an explicit code — + // never a quiet ok (indistinguishable from "the tree is clean" for a gate) and never an + // unhandled TypeError, which would read as an infrastructure fault rather than a verdict. + if (!Array.isArray(changedPaths)) { + add(FAMILY_REASON.BAD_CHANGED_PATHS); + return { ok: false, errors }; + } + if (!Array.isArray(derived) || derived.some((f) => !f || typeof f.name !== 'string')) { + add(FAMILY_REASON.DERIVED_UNUSABLE); + return { ok: false, errors }; + } + if (!Array.isArray(fixtures) || fixtures.some((n) => typeof n !== 'string')) { + add(FAMILY_REASON.FIXTURES_UNUSABLE); + return { ok: false, errors }; + } + if (baseline === null || baseline === undefined || typeof baseline !== 'object' || Array.isArray(baseline)) { + add(FAMILY_REASON.BASELINE_UNUSABLE); + return { ok: false, errors }; + } + if (current === null || current === undefined || typeof current !== 'object' || Array.isArray(current)) { + add(FAMILY_REASON.CURRENT_UNUSABLE); + return { ok: false, errors }; + } + + const derivedNames = new Set(derived.map((f) => f.name)); + const fixtureNames = new Set(fixtures); + const baselineNames = new Set(Object.keys(baseline)); + const currentNames = new Set(Object.keys(current)); + + // The floor. Independent of every derivation, so a uniformly shrunken universe cannot + // satisfy it by moving both sides together. + if (derivedNames.size < minimum) add(FAMILY_REASON.BELOW_FLOOR); + + // Single-tree drift: the registry and the recorded fixtures must describe one world. + for (const name of fixtureNames) { + if (!derivedNames.has(name)) add(FAMILY_REASON.FIXTURE_WITHOUT_RUNTIME, name); + } + for (const name of derivedNames) { + if (!fixtureNames.has(name)) add(FAMILY_REASON.RUNTIME_WITHOUT_FIXTURE, name); + } + + // #2086: claude's local-scope layout is a family in its own right and was once dropped + // from both sides at once. Pinned by name on both, never inferred from a total. + if (!currentNames.has('claude-local')) add(FAMILY_REASON.MISSING_CLAUDE_LOCAL, 'claude-local'); + if (!baselineNames.has('claude-local')) add(FAMILY_REASON.MISSING_CLAUDE_LOCAL, 'claude-local'); + + // Cross-tree set difference, both directions, with ONE permission path: the PR + // plausibly touched the runtime registry. Symmetric on purpose — an ack-style bypass on + // only one side would make removals easier to wave through than additions, and the + // drift-ack file exists for unattributable emitted-PATH deltas, not for family churn. + const attributed = touchesRuntimeRegistry(changedPaths); + + if (!attributed) { + for (const name of currentNames) { + if (!baselineNames.has(name)) add(FAMILY_REASON.ADDED_UNATTRIBUTED, name); + } + for (const name of baselineNames) { + if (!currentNames.has(name)) add(FAMILY_REASON.DROPPED_UNATTRIBUTED, name); + } + } + + return { ok: errors.length === 0, errors }; +} + /** Bounded git invocation. CLAUDE.md → KNOWN DEFECTS: every git subprocess needs a * timeout (5-30s); an unbounded execFileSync is an indefinite hang, and it is how * macOS CI silently stops reporting. */ @@ -151,6 +307,35 @@ function resolveBase(env = process.env) { return null; } +/** + * Family names present in the fixture directory AT `base`. + * + * Enumerated from the ref itself, NOT from `MANIFEST_FAMILIES` — that constant is + * imported at module load and therefore describes PR HEAD's registry. Deriving the + * baseline from it makes a REMOVED runtime invisible: the name is already gone from the + * current registry, so the loop never asks the base ref for it, `baseline` silently omits + * a family that genuinely existed, and the dropped-family check can never fire. Asking + * the ref what it actually contains is the only way the "before" side is really "before". + */ +function baselineFamilyNamesAtRef(base, { cwd = REPO_ROOT } = {}) { + let out; + try { + out = git(['ls-tree', '--name-only', base, `${FIXTURE_SUBDIR}/`], { cwd }); + } catch { + return []; // fixtures absent at that ref (e.g. after Phase 4's cutover) + } + return out + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.endsWith('.json')) + .map((line) => line.slice(line.lastIndexOf('/') + 1).replace(/\.json$/, '')) + // These names become object keys below. They now come from git output rather than a + // trusted constant, so a fixture committed as `__proto__.json` would turn + // `manifests[name] = parsed` into a prototype write. Compared inline (not via a Set) + // because that is the form the prototype-pollution analysis recognizes. + .filter((name) => name !== '__proto__' && name !== 'constructor' && name !== 'prototype'); +} + /** * Emitted manifest set at `base`, read from the committed fixtures at that ref. * Returns null when the fixtures are absent at `base` (i.e. after Phase 4's cutover), @@ -159,7 +344,7 @@ function resolveBase(env = process.env) { function baselineManifestsAtRef(base = 'origin/next') { const manifests = {}; let found = 0; - for (const { name } of MANIFEST_FAMILIES) { + for (const name of baselineFamilyNamesAtRef(base)) { let raw; try { raw = git(['show', `${base}:${FIXTURE_SUBDIR}/${name}.json`]); @@ -255,12 +440,18 @@ module.exports = { ACK_PATH, FIXTURE_SUBDIR, MANIFEST_FAMILIES, + MINIMUM_MANIFEST_FAMILIES, + REGISTRY_SIGNAL_PATHS, + FAMILY_REASON, + touchesRuntimeRegistry, + reconcileFamilies, GIT_TIMEOUT_MS, git, resolveChangedPaths, resolveBaseSha, baseRefCandidates, resolveBase, + baselineFamilyNamesAtRef, baselineManifestsAtRef, baselineSizesAtRef, currentManifests, diff --git a/tests/helpers/install-shared.cjs b/tests/helpers/install-shared.cjs index d7f316eb5..13f9093d0 100644 --- a/tests/helpers/install-shared.cjs +++ b/tests/helpers/install-shared.cjs @@ -72,6 +72,42 @@ const RUNTIME_META = { zcode: { localDir: '.zcode', globalSuffix: '.zcode' }, }; +/** + * The emitted manifest families, as (fixtureName -> install spec). + * + * NOT simply `Object.keys(RUNTIME_META)`: that has 18 entries while the fixture set has + * 19. The extra one is `claude-local` — claude is the reference host and the ONLY + * runtime with a distinct LOCAL "legacy flat-commands" layout (`commands/gsd-*.md` + + * `agents/gsd-*.md` at project scope), which `golden-install-parity.test.cjs` guards + * with a hand-coded test outside its RUNTIME_META loop (#2086). + * + * Enumerating from RUNTIME_META alone dropped that family from BOTH sides of the + * differential, so a same-count self-check (18 === 18) passed vacuously and a PR + * changing Claude's local-scope output would fail the golden while the attribution + * check reported ok. + * + * Lives HERE, beside RUNTIME_META, so the emitted-attribution helpers and the + * emitted-provenance table read ONE derivation rather than each carrying a literal. + * Two surfaces sharing a hand-maintained count is what produced the #2723 deadlock: + * a single constant was asserted against both the base ref and the PR head, which + * legitimately differ whenever a PR adds or removes a runtime. + */ +const MANIFEST_FAMILIES = [ + ...Object.keys(RUNTIME_META).map((runtime) => ({ name: runtime, runtime, scope: 'global' })), + { name: 'claude-local', runtime: 'claude', scope: 'local' }, +]; + +/** + * Absolute floor on the family set, independent of any derivation. + * + * A pure equality between "derived" and "recorded" cannot catch a universe that shrank + * on BOTH sides at once (drop a RUNTIME_META entry and delete its fixture together, and + * 18 === 18 passes over a smaller world). This floor is the one number that must not be + * derived — it ratchets, and lowering it is a deliberate, reviewable act. It never + * blocks ADDING a runtime, which is the asymmetry the old shared literal lacked. + */ +const MINIMUM_MANIFEST_FAMILIES = 19; + // Runtimes that emit per-skill files under skills/ (not rules-based or commands-based) const SKILL_RUNTIMES = [ 'claude', 'opencode', 'kilo', 'codex', 'copilot', 'antigravity', @@ -396,6 +432,8 @@ module.exports = { EXPECTED_SH_HOOKS, EXPECTED_ALL_HOOKS, RUNTIME_META, + MANIFEST_FAMILIES, + MINIMUM_MANIFEST_FAMILIES, SKILL_RUNTIMES, PKG_VERSION, VOLATILE_FILES,