Files
msd-core/tests/helpers/emitted-diff.cjs
Tom Boucher 9138271b5f test(#2723): differential emitted-attribution check, dual-run beside the golden (#2737)
* test(#2723): differential emitted-attribution check, dual-run beside the golden

Phase 3 of #2719. The conservation law itself, running BESIDE
golden-install-parity.test.cjs -- both green, fixtures untouched.

Every emitted path whose hash moved between next HEAD and PR HEAD must be
attributable, through the Phase 2 table, to a path the PR actually changed.
Unattributable deltas fail with the paths NAMED. The only way through is a
committed acknowledgment, never a flag -- a contributor facing a red gate
sets a flag, which is what UPDATE_GOLDEN=1 is today.

The central decision is that the law is a PURE function (no fs, git,
installer, or clock), with I/O confined to a separate resolver. The naive
one-big-integration-test shape would need ~38 installer spawns per assertion,
so #2723's four failing-first criteria would not in practice have been
written -- which is exactly how a phase ships promised-but-not-built. Pure,
they are millisecond table tests, and the Stryker gate can actually bite.

Buckets are conserved: every moved path lands in exactly one of
attributed | unattributable | acked, property-tested at 400 runs. A path the
provenance table cannot resolve surfaces as an error, never a silent skip.

Asymmetries that are deliberate, each with a test:
- an ADDED emitted key is a ripple too, not just a modified one
- synthesized paths are exempt; code-derived ones are NOT (Phase 2 refused to
  mark them exempt precisely because exempt means permanently blind)
- shrinkage needs no ack; growth does. Gating shrinkage would punish exactly
  what the size ratchet wants
- a STALE ack is a hard failure -- an ack outliving its ripple pre-clears the
  next one on that path
- a failed `git diff` is an explicit error, never an empty changedPaths set;
  reading it as "nothing changed" would make everything unattributable and
  produce a failure storm that reads like a real finding
- prefix sources are SEGMENT-aware, so `agents/` does not attribute
  `agentsfoo/x.md`

Baseline is cached, not committed, keyed on the next sha. A stale key is
refused rather than used: absence fails loudly and gets fixed, whereas
staleness produces a confident wrong answer. An explicitly pointed-at
GSD_EMITTED_BASELINE that is stale is a hard stop; a stale cache falls
through to the in-job build. No baseline-unavailable path returns -- ADR-2719
section 6 names that trap, since in node:test a bare return is a PASS.

Both the conservation property and the staleness gate were mutation-verified
(injecting a swallowed key fails 9 tests; disabling the staleness comparison
fails 5).

Fixtures, generators, the merge driver and the ADR status are untouched --
those are Phase 4 (#2724).

Refs #2719

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

* test(#2723): compute stale acks once, after the size pass

Self-review defect found while the reviewers were running. `staleAcks` was
computed twice: once between the hash pass and the size pass, then again
after. Only the second value was returned, so the first was dead code -- and
the dead one was placed where it would have been WRONG.

An acknowledgment can be consumed by either a hash move or a size growth.
Computing staleness before the size pass reports a legitimate growth ack as
stale, which is a false failure that pushes a contributor to delete the very
ack that is doing its job.

Now computed once, after both passes, with a regression test. Verified by
mutation: restoring the early computation fails 2 tests.

Refs #2719

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

* test(#2723): wire the attribution check to the real tree, not just synthetic input

An isolated reviewer caught that the first cut was INTERFACE-ONLY: nothing
read the ack file from disk, nothing shelled git, nothing built real
manifests. Every test was true of hand-built inputs and none of the repo, so
the acceptance criterion "both this check and golden-install-parity green on
the same tree" was trivially true rather than meaningfully true. That is the
promised-but-not-built failure this epic keeps finding in its predecessors,
recurring one phase later for the wiring itself. Taken, not argued.

Adds tests/helpers/emitted-runtime.cjs -- the only module that touches git,
disk, or the installer -- and an integration test that runs the same pure law
against reality:

- CURRENT side: 19 real installer spawns via runMinimalInstall +
  buildParityManifest, the same machinery the golden harness uses.
- BASELINE side: `git show origin/next:<fixture>`. That is next's RECORDED
  emitted state and it costs nothing. Deliberately NOT the working-tree
  fixtures, which are whatever this PR's author regenerated -- comparing
  against those would be vacuous. Phase 4 deletes the fixtures and swaps in
  resolveBaseline's cache path, already implemented and tested.
- changed paths from real `git diff --name-only origin/next...HEAD`, with the
  git subprocess bounded at 30s per CLAUDE.md's unbounded-subprocess rule.
- the real tests/emitted-drift-ack.json (absent is legal; present-but-empty
  or unparseable throws rather than being read as absent).

Verified it can actually fail: an uncommitted edit to a shipped workflow
moves emitted output but never appears in the committed diff, and the check
names all 18 affected emitted paths with the message format ADR-2719 §1
specifies. Restores clean.

Also from review:
- readAckFile now has a real test exercising the SUT across absent / valid /
  empty / unparseable / unreadable. The previous test asserted fs behaviour
  rather than SUT behaviour, because no SUT ack-reading path existed yet.
- formatReport's sampleLimit gains true limit-1/limit/limit+1 coverage at
  19/20/21. A test was previously NAMED "(limit+1)" while testing no numeric
  limit at all, which is worse than no coverage because it reads as covered.

Windows uses an explicit t.skip (install output is platform-specific there,
mirroring the golden harness) -- never a bare return, which node:test scores
as a PASS.

Refs #2719

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

* test(#2723): cover the claude-local manifest family in the real-tree check

Isolated adversarial review, MAJOR. The real-tree wiring enumerated
Object.keys(RUNTIME_META) -- 18 entries -- while the emitted manifest set has
19 families. The 19th 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).

The family was dropped from BOTH sides, so the test's own self-check
(current.length === baseline.length) passed vacuously at 18 === 18. A PR
changing Claude's local-scope output would have failed the golden while this
check reported ok -- and that disagreement is precisely what the dual-run
window is designed to surface as a provenance-table hole. A wiring omission
masquerading as one is the worst available failure here, because it would
have been read as evidence about Phase 2 rather than a bug in Phase 3.

Fixed by deriving MANIFEST_FAMILIES explicitly (18 global + claude-local at
local scope) instead of inferring the set from RUNTIME_META.

The self-check is also repaired: it now asserts both sides against the
INDEPENDENT EXPECTED_MANIFEST_COUNT from the Phase 2 table, and asserts
claude-local specifically. Comparing the two sides to each other can never
catch a family missing from both -- the assertion has to come from outside.
Verified by mutation: removing claude-local again fails the test.

Also from the same review:
- sourceSatisfiedBy returns the matched source string, so an empty-string
  source would return '' and the caller's `if (hit)` would silently discard a
  real match. Unreachable today (every rule source is a non-empty template)
  but a footgun for the next rule author; now `!== null`.
- the purity fixture used a single-element changedPaths array, so an in-place
  sort would have been invisible. Now three elements in unsorted order.

Refs #2719

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

* fix(#2723): resolve the base ref tolerantly instead of hard-requiring origin/next

The first matrix run failed on both linux lanes:

  differential attribution over the real tree
  cannot resolve origin/next (Command failed: git rev-parse origin/next)

Not a flake, and not an environment excuse -- a real defect in this diff. The
gsd-test runner shallow-clones and merges base+head, so no origin/* remote-
tracking refs exist in the container. My own fail-loud path fired correctly;
what was wrong was hard-depending on that ref existing. GitHub Actions has the
same shape by default, which is exactly why changeset-required.yml carries an
explicit `git fetch origin "${BASE_REF}:refs/remotes/origin/${BASE_REF}"`.

Now resolved through an ordered candidate list -- GSD_EMITTED_BASE (explicit
lane override), then origin/$GITHUB_BASE_REF and $GITHUB_BASE_REF, then
origin/next and next -- de-duplicated, each verified with
`rev-parse --verify <ref>^{commit}`.

When NO candidate resolves the test takes an explicit t.skip() naming every
ref it tried and stating that the gate did not run here. That is the
ADR-2719 section 6 distinction: t.skip is REPORTED as skipped, whereas a bare
return is scored as a PASS. Hard-failing was the other option and is wrong --
it would make the suite permanently red wherever a base ref cannot exist by
construction, which is a statement about the checkout, not a propagation
finding.

The candidate ordering is pinned by a unit test rather than left implicit,
since the ordering IS the fix.

Refs #2719

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5kQs6ZufZDySC6zDJfYP6

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 23:22:37 -04:00

320 lines
12 KiB
JavaScript

'use strict';
/**
* Differential emitted-artifact attribution — the conservation law (ADR-2719 §1,
* issue #2723, epic #2719 Phase 3).
*
* Given the emitted manifests at `next` HEAD and at PR HEAD, plus the repo paths the
* PR actually changed, decide which moved emitted paths are EXPLAINED by the diff and
* which are not. Unattributable deltas are a hard failure that names them; the only
* way through is a committed acknowledgment (`tests/emitted-drift-ack.json`).
*
* ── Why this module is pure ──────────────────────────────────────────────────
* No fs, no git, no installer, no clock. The naive shape — one integration test that
* builds 19 manifests at each end and asserts — cannot practically exercise the four
* failing-first criteria #2723 requires, so in practice they would not get written,
* which is precisely how a phase ships promised-but-not-built. Keeping the law pure
* makes every criterion a millisecond-scale table test, and makes the Stryker gate
* able to bite (a 20-branch pure function is mutation-testable; a 40-minute
* integration test is not).
*
* The expensive part — obtaining the manifests — lives in emitted-baseline.cjs.
*
* ── What this does NOT do ────────────────────────────────────────────────────
* It never re-derives a byte (ADR-2719 §1 is explicit that asserting
* `emitted == transform(source)` is the tautology ADR-2264's Amendment rejected).
* It constrains which keys may move. It also never mutates repo state: no
* regeneration, no auto-ack. `UPDATE_GOLDEN=1` is exactly the escape hatch this
* design removes.
*/
const { attributeEmittedPath } = require('./emitted-provenance.cjs');
/** Ack schema version. Pinned from day one: contributors hand-write this file, so its
* shape is public the moment it ships (Hyrum). Loosening later is easy; tightening is not. */
const ACK_VERSION = 1;
/**
* Does a changed repo path satisfy a provenance `sources` entry?
*
* A trailing `/` marks a PREFIX (Phase 2's SOURCE_PREFIX_SUFFIX contract) — e.g. Kimi's
* root agent aggregates all of `agents/`. Prefix matching is SEGMENT-AWARE on purpose:
* a bare `startsWith('agents/')` would also accept `agentsfoo/x.md` under a source of
* `agents`, and silently over-attribute. Exact entries compare exactly.
*/
function sourceSatisfiedBy(source, changedSet) {
if (source.endsWith('/')) {
for (const changed of changedSet) {
if (changed.startsWith(source)) return changed;
}
return null;
}
return changedSet.has(source) ? source : null;
}
/**
* Normalize + validate the acknowledgment document.
*
* Rejects a document that parses but is not a plain object. Treating `0` / `"s"` / `[]` /
* `null` / `true` as "no acks" would SILENTLY DISARM the gate — the single worst failure
* available here, because it looks identical to a healthy run.
*
* @returns {{ entries: Map<string, {reason: string, runtime?: string}>, errors: string[] }}
*/
function parseAck(doc, { source = 'emitted-drift-ack.json' } = {}) {
const errors = [];
const entries = new Map();
if (doc === null || doc === undefined) return { entries, errors }; // absent == no acks (legal)
if (typeof doc !== 'object' || Array.isArray(doc)) {
errors.push(
`${source}: must be a JSON object, got ${Array.isArray(doc) ? 'array' : typeof doc}`,
);
return { entries, errors };
}
if (doc.version !== undefined && doc.version !== ACK_VERSION) {
errors.push(`${source}: unsupported version ${JSON.stringify(doc.version)} (expected ${ACK_VERSION})`);
}
const paths = doc.paths;
if (paths === undefined) return { entries, errors }; // `{}` or `{version:1}` == no acks
if (paths === null || typeof paths !== 'object' || Array.isArray(paths)) {
errors.push(`${source}: "paths" must be an object of <emitted path> -> { reason }`);
return { entries, errors };
}
for (const [rel, value] of Object.entries(paths)) {
const reason = value && typeof value === 'object' ? value.reason : value;
if (typeof reason !== 'string' || reason.trim() === '') {
// "name them AND say why" is the contract (ADR-2719 §3). An ack with no reason
// is a silent regeneration wearing a declaration's clothes.
errors.push(`${source}: ack for "${rel}" has no non-empty "reason"`);
continue;
}
entries.set(rel, {
reason: reason.trim(),
runtime: value && typeof value === 'object' ? value.runtime : undefined,
});
}
return { entries, errors };
}
/**
* The conservation law.
*
* @param {object} opts
* @param {object} opts.baseline { [runtime]: { [rel]: hash } } at `next` HEAD
* @param {object} opts.current { [runtime]: { [rel]: hash } } at PR HEAD
* @param {string[]} opts.changedPaths repo paths the PR changed (git diff --name-only)
* @param {object} [opts.ack] parsed emitted-drift-ack.json document (or null)
* @param {object} [opts.sizeBaseline] { [name]: bytes } workflow/agent sizes at next
* @param {object} [opts.sizeCurrent] { [name]: bytes } workflow/agent sizes at PR HEAD
*
* @returns {{
* moved: number, attributed: Array, unattributable: Array, acked: Array,
* removed: Array, grown: Array, shrunk: Array, staleAcks: string[], errors: string[], ok: boolean
* }}
*/
function diffEmitted({
baseline,
current,
changedPaths,
ack = null,
sizeBaseline = null,
sizeCurrent = null,
} = {}) {
const errors = [];
if (!baseline || typeof baseline !== 'object' || Array.isArray(baseline)) {
errors.push('baseline manifest set must be an object keyed by runtime');
}
if (!current || typeof current !== 'object' || Array.isArray(current)) {
errors.push('current manifest set must be an object keyed by runtime');
}
if (!Array.isArray(changedPaths)) {
// NOT the same as an empty array. A failed `git diff` must never be read as
// "nothing changed" — that would make every moved hash unattributable and produce
// a failure storm that reads like a real finding.
errors.push('changedPaths must be an array (a failed git diff is an error, not an empty set)');
}
if (errors.length) {
return {
moved: 0, attributed: [], unattributable: [], acked: [], removed: [],
grown: [], shrunk: [], staleAcks: [], errors, ok: false,
};
}
const changedSet = new Set(changedPaths);
const { entries: ackEntries, errors: ackErrors } = parseAck(ack);
errors.push(...ackErrors);
const attributed = [];
const unattributable = [];
const acked = [];
const removed = [];
const usedAcks = new Set();
let moved = 0;
const runtimes = new Set([...Object.keys(baseline), ...Object.keys(current)]);
for (const runtime of [...runtimes].sort()) {
const before = baseline[runtime] || {};
const after = current[runtime] || {};
const keys = new Set([...Object.keys(before), ...Object.keys(after)]);
for (const rel of [...keys].sort()) {
const had = Object.prototype.hasOwnProperty.call(before, rel);
const has = Object.prototype.hasOwnProperty.call(after, rel);
if (had && has && before[rel] === after[rel]) continue; // unchanged — ignored
const change = !had ? 'added' : (!has ? 'removed' : 'modified');
moved++;
let attribution;
try {
attribution = attributeEmittedPath(rel, runtime);
} catch (err) {
// A path the Phase 2 table cannot resolve is surfaced, never silently skipped —
// otherwise a table hole becomes a blind spot in the differential too.
errors.push(`${runtime}: ${rel}: ${err.message}`);
continue;
}
const record = { runtime, rel, change, ruleId: attribution.ruleId, kind: attribution.kind };
if (change === 'removed') removed.push(record);
// Synthesized paths carry no repo source by definition, so a delta in them can
// never be "unexplained by the diff" — exempt, but still counted and reported.
if (attribution.kind === 'synthesized') {
attributed.push({ ...record, via: '<synthesized: exempt>' });
continue;
}
let via = null;
for (const source of attribution.sources) {
const hit = sourceSatisfiedBy(source, changedSet);
// `!== null`, not truthiness: an exact match returns the source string, and an
// empty-string source would return '' — falsy, so a real match would be
// silently discarded. Unreachable with today's rules (every source is a
// non-empty template) but it is a footgun for the next rule author.
if (hit !== null) { via = hit; break; }
}
if (via !== null) {
attributed.push({ ...record, via });
} else if (ackEntries.has(rel)) {
usedAcks.add(rel);
acked.push({ ...record, reason: ackEntries.get(rel).reason });
} else {
unattributable.push({ ...record, expectedSources: attribution.sources });
}
}
}
// ── Size ratchet, folded into the same machine (ADR-2719 §4, must-have 6) ──
// NOTE: stale-ack detection is computed AFTER this block, not before. An ack may be
// consumed by either a hash move or a size growth, so computing it earlier would
// report a size-growth ack as stale.
const grown = [];
const shrunk = [];
if (sizeBaseline && sizeCurrent) {
for (const name of Object.keys(sizeCurrent).sort()) {
if (!Object.prototype.hasOwnProperty.call(sizeBaseline, name)) continue;
const from = sizeBaseline[name];
const to = sizeCurrent[name];
if (to > from) {
// Growth needs the SAME acknowledgment. Anti-creep survives without pinning a
// number: "verify-work.md grew 1,247 bytes" beats a number moving in a 93-line map.
const isAcked = ackEntries.has(name);
if (isAcked) usedAcks.add(name);
grown.push({ name, from, to, delta: to - from, acked: isAcked });
} else if (to < from) {
// Shrinkage is not creep — reported, never gated.
shrunk.push({ name, from, to, delta: from - to });
}
}
}
const unackedGrowth = grown.filter((g) => !g.acked);
// An ack that outlives the ripple it explained is future blindness: it would silently
// pre-clear a NEW ripple on the same path. It must be deleted when the ripple is.
// Computed here, once, after BOTH the hash pass and the size pass have consumed acks.
const staleAcks = [...ackEntries.keys()].filter((rel) => !usedAcks.has(rel)).sort();
const ok = errors.length === 0
&& unattributable.length === 0
&& unackedGrowth.length === 0
&& staleAcks.length === 0;
return {
moved,
attributed,
unattributable,
acked,
removed,
grown,
shrunk,
staleAcks,
errors,
ok,
};
}
/**
* Render a report as the failure message ADR-2719 §1 specifies — it sells the whole
* design on this text, so it is a deliverable, not a detail.
*/
function formatReport(result, { sampleLimit = 20 } = {}) {
const parts = [];
if (result.errors.length) {
parts.push(`${result.errors.length} error(s):\n ${result.errors.slice(0, sampleLimit).join('\n ')}`);
}
if (result.unattributable.length) {
const list = result.unattributable.slice(0, sampleLimit)
.map((u) => ` ${u.runtime}: ${u.rel}\n rule ${u.ruleId}; expected a change under ${u.expectedSources.join(' or ')}`);
parts.push(
`${result.unattributable.length} emitted path(s) changed that nothing in this diff explains:\n${list.join('\n')}` +
(result.unattributable.length > sampleLimit
? `\n …and ${result.unattributable.length - sampleLimit} more`
: '') +
'\n\nIf this ripple is intended, record it in tests/emitted-drift-ack.json naming each\n' +
'path and why. Do NOT regenerate anything to silence this.',
);
}
const unackedGrowth = result.grown.filter((g) => !g.acked);
if (unackedGrowth.length) {
const list = unackedGrowth.slice(0, sampleLimit)
.map((g) => ` ${g.name} grew ${g.delta} bytes (${g.from} -> ${g.to})`);
parts.push(
`${unackedGrowth.length} file(s) grew without an acknowledgment:\n${list.join('\n')}`,
);
}
if (result.staleAcks.length) {
parts.push(
`${result.staleAcks.length} stale acknowledgment(s) — the ripple they explained is gone, ` +
'so they must be deleted (an ack that outlives its ripple pre-clears the next one):\n ' +
result.staleAcks.slice(0, sampleLimit).join('\n '),
);
}
return parts.join('\n\n');
}
module.exports = {
ACK_VERSION,
sourceSatisfiedBy,
parseAck,
diffEmitted,
formatReport,
};