Files
msd-core/tests/helpers/emitted-runtime.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

270 lines
10 KiB
JavaScript

'use strict';
/**
* Real-world I/O shell for the differential attribution check (#2723, ADR-2719).
*
* `emitted-diff.cjs` holds the pure conservation law; this module is the only place
* that touches git, the filesystem, or the installer. Keeping them apart is what makes
* the law's acceptance criteria testable in milliseconds — but the shell still has to
* exist and actually run, or the phase ships as interface-only, which an isolated
* reviewer correctly called out on the first cut of this work.
*
* ── Baseline source during the dual-run window ───────────────────────────────
* The baseline is the emitted manifest set at `next` HEAD. During Phase 3 that is
* available for FREE and for REAL via `git show origin/next:<fixture>` — the committed
* golden fixtures ARE next's recorded emitted state, and CI keeps them current there.
* No worktree, no rebuild, no 19 installer spawns for the baseline side.
*
* Critically this is NOT the same as reading the fixtures from the WORKING TREE: those
* are whatever the PR author regenerated, so comparing against them would be vacuous
* (current vs. the author's own regeneration). Reading them at `origin/next` is what
* makes the comparison a real differential against upstream state.
*
* Phase 4 (#2724) deletes the fixtures, at which point `resolveBaseline`'s cache path
* (already implemented and tested in emitted-baseline.cjs) becomes the source. That
* swap is the only change Phase 4 needs here.
*
* The CURRENT side is built for real — 19 installer spawns via the same
* `runMinimalInstall` + `buildParityManifest` the golden harness uses. It is the
* expensive half on purpose: if a PR forgot to regenerate, current-real differs from
* next's recorded state and the attribution actually runs, which is the whole point.
*/
const fs = require('node:fs');
const path = require('node:path');
const { execFileSync } = require('node:child_process');
const { cleanup } = require('../helpers.cjs');
const {
RUNTIME_META,
runMinimalInstall,
buildParityManifest,
} = require('./install-shared.cjs');
const REPO_ROOT = path.join(__dirname, '..', '..');
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).
*
* 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 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.
*/
const MANIFEST_FAMILIES = [
...Object.keys(RUNTIME_META).map((runtime) => ({ name: runtime, runtime, scope: 'global' })),
{ name: 'claude-local', runtime: 'claude', scope: 'local' },
];
/** 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. */
const GIT_TIMEOUT_MS = 30_000;
function git(args, { cwd = REPO_ROOT } = {}) {
return execFileSync('git', args, {
cwd,
encoding: 'utf8',
timeout: GIT_TIMEOUT_MS,
maxBuffer: 64 * 1024 * 1024,
stdio: ['ignore', 'pipe', 'pipe'],
});
}
/**
* Repo paths the PR changed, via the three-dot form so the comparison is against the
* merge base rather than the tip of `base`.
*
* A git failure THROWS. It must never degrade to an empty array: reading "git broke" as
* "nothing changed" would make every moved hash unattributable and produce a failure
* storm that reads exactly like a real finding.
*/
function resolveChangedPaths(base = 'origin/next') {
let out;
try {
out = git(['diff', '--name-only', `${base}...HEAD`]);
} catch (err) {
throw new Error(
`emitted-attribution: could not resolve changed paths from "${base}...HEAD": ${err.message}. ` +
'This is a hard error on purpose — treating it as "no changes" would mark every ' +
'moved emitted path unattributable.',
);
}
return out.split('\n').map((l) => l.trim()).filter(Boolean);
}
/** Resolve `base` to a 40-hex sha, for the baseline cache-key discipline (ADR §5). */
function resolveBaseSha(base = 'origin/next') {
return git(['rev-parse', base]).trim();
}
/**
* Base-ref candidates, most-specific first.
*
* The differential needs a ref for `next`, and that ref is NOT universally present:
* - the gsd-test runner shallow-clones and merges base+head, so no `origin/*`
* remote-tracking refs exist in the container (verified: `git rev-parse
* origin/next` fails there, which is what turned this test red on its first run);
* - GitHub Actions' checkout does not create remote-tracking branches for OTHER
* branches by default, which is exactly why `changeset-required.yml` carries an
* explicit `git fetch origin "${BASE_REF}:refs/remotes/origin/${BASE_REF}"` step.
*
* `GSD_EMITTED_BASE` lets a lane name the ref (or sha) directly. `GITHUB_BASE_REF` is
* set by Actions on pull_request events.
*/
function baseRefCandidates(env = process.env) {
const candidates = [];
if (env.GSD_EMITTED_BASE) candidates.push(env.GSD_EMITTED_BASE);
if (env.GITHUB_BASE_REF) {
candidates.push(`origin/${env.GITHUB_BASE_REF}`, env.GITHUB_BASE_REF);
}
candidates.push('origin/next', 'next');
return [...new Set(candidates)];
}
/**
* First candidate base ref that actually resolves, or null when none do.
*
* Returning null is NOT a pass — the caller turns it into an explicit `t.skip()` with
* the full candidate list in the message, so an environment where the gate did not run
* says so out loud. A bare `return` there would be a PASS (ADR-2719 §6), and a hard
* failure would make the suite permanently red in the gsd-test container, where no
* base ref can exist by construction.
*/
function resolveBase(env = process.env) {
for (const candidate of baseRefCandidates(env)) {
try {
const sha = git(['rev-parse', '--verify', `${candidate}^{commit}`]).trim();
if (/^[0-9a-f]{40}$/.test(sha)) return { ref: candidate, sha };
} catch { /* try the next candidate */ }
}
return null;
}
/**
* 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),
* which is the signal to fall back to `resolveBaseline`'s cache path.
*/
function baselineManifestsAtRef(base = 'origin/next') {
const manifests = {};
let found = 0;
for (const { name } of MANIFEST_FAMILIES) {
let raw;
try {
raw = git(['show', `${base}:${FIXTURE_SUBDIR}/${name}.json`]);
} catch {
continue; // absent at that ref
}
let parsed;
try {
parsed = JSON.parse(raw);
} catch (err) {
throw new Error(`emitted-attribution: ${base}:${FIXTURE_SUBDIR}/${name}.json is not valid JSON: ${err.message}`);
}
if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) {
throw new Error(`emitted-attribution: ${base}:${FIXTURE_SUBDIR}/${name}.json must be an object of path->hash`);
}
manifests[name] = parsed;
found++;
}
return found === 0 ? null : manifests;
}
/** Size maps at `base`, for the ratchet half. Null when absent at that ref. */
function baselineSizesAtRef(base = 'origin/next') {
const sizes = {};
let found = 0;
for (const rel of ['tests/workflow-size-baseline.json', 'tests/agent-size-baseline.json']) {
try {
const parsed = JSON.parse(git(['show', `${base}:${rel}`]));
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
Object.assign(sizes, parsed);
found++;
}
} catch { /* absent at that ref */ }
}
return found === 0 ? null : sizes;
}
/**
* Build the CURRENT emitted manifest set for real — one installer spawn per runtime.
* This is the expensive, honest half: it reflects what the tree actually emits now,
* not what the author regenerated into a fixture.
*/
function currentManifests() {
const manifests = {};
for (const { name, runtime, scope } of MANIFEST_FAMILIES) {
const { configDir, root } = runMinimalInstall({ runtime, scope });
try {
manifests[name] = buildParityManifest(configDir, root);
} finally {
cleanup(root);
}
}
return manifests;
}
/** Current on-disk sizes for the workflow + agent families the ratchet covers. */
function currentSizes() {
const sizes = {};
for (const [dir, filter] of [
[path.join(REPO_ROOT, 'gsd-core', 'workflows'), (f) => f.endsWith('.md')],
[path.join(REPO_ROOT, 'agents'), (f) => f.endsWith('.md')],
]) {
if (!fs.existsSync(dir)) continue;
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
if (!entry.isFile() || !filter(entry.name)) continue;
sizes[entry.name] = fs.statSync(path.join(dir, entry.name)).size;
}
}
return sizes;
}
/**
* Read `tests/emitted-drift-ack.json`.
* Absent is legal and means "no acks" — its PRESENCE is the alarm (ADR §3).
* A present-but-unreadable or unparseable file THROWS: silently treating it as absent
* would disarm the gate in the one case where someone is actively using it.
*/
function readAckFile(ackPath = ACK_PATH) {
if (!fs.existsSync(ackPath)) return null;
const raw = fs.readFileSync(ackPath, 'utf8');
if (raw.trim() === '') {
throw new Error(`emitted-attribution: ${path.basename(ackPath)} is present but empty`);
}
try {
return JSON.parse(raw);
} catch (err) {
throw new Error(`emitted-attribution: ${path.basename(ackPath)} is not valid JSON: ${err.message}`);
}
}
module.exports = {
REPO_ROOT,
ACK_PATH,
FIXTURE_SUBDIR,
MANIFEST_FAMILIES,
GIT_TIMEOUT_MS,
git,
resolveChangedPaths,
resolveBaseSha,
baseRefCandidates,
resolveBase,
baselineManifestsAtRef,
baselineSizesAtRef,
currentManifests,
currentSizes,
readAckFile,
};