Files
msd-core/tests/mutation-workflow-base-ref.test.cjs
Tom Boucher 3eb1cede26 fix(#1880): distinguish a corrupt config from an absent one (epic #1879 Phase 1) (#2688)
* test(#1880): prove corrupt config is indistinguishable from absent

Failing-first. Encodes the issue's runtime repro: a trailing comma in
.planning/config.json currently yields source:builtin-defaults with
degraded:false - byte-identical to the file not existing - and the user's
entire configuration is silently discarded.

Asserts on the typed surface (CONFIG_REASON, _warnedUnusableConfig) rather
than diagnostic prose, per the ADR-1411 amendment's test-methodology clause
and CONTRIBUTING.md's raw-text-matching rule. IO failure is injected by
monkeypatching fs.readFileSync and restoring in t.after(), never chmod 0o000
(root bypasses mode bits).

Refs #1879

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

* fix(#1880): distinguish a corrupt config from an absent one

loadConfigResolved wrapped the read, the JSON.parse and the entire config
build in one try with one catch, so ENOENT, EACCES and SyntaxError all fell
through to the same defaults and the branches returned degraded:false -
actively asserting health over discarded configuration. A single trailing
comma in .planning/config.json silently replaced the user's whole config,
reporting source:builtin-defaults degraded:false, byte-identical to having
no config file at all.

ConfigResolution now carries a machine-readable reason. Genuine absence keeps
degraded:false / not_configured; a file that exists but cannot be used sets
degraded:true with config_unparseable or config_unreadable. The same split
applies to the root config and to ~/.gsd/defaults.json.

Control flow is deliberately unchanged. preflight_check reports cyclomatic
141 / cognitive 196 and 93 dependents on this function, with the guidance
that small edits beat one big one, so faults are CAPTURED at the existing
read sites and stamped onto the returns rather than the try/catch being
restructured.

Also carries the ADR-1411 amendment's wiring clause: loadConfig returns
.config alone to ~51 call sites and would never see the new field, so an
unusable file emits a deduplicated stderr diagnostic keyed on resolved path
plus errno. Without it the reason would be an unreachable field and the user
whose config was discarded would still get no signal - the actual defect.

Registers the config-loader seam in lint-resolution-provenance, which until
now guarded only agent-skills.

Caller audit: ConfigResolution.degraded has exactly one consumer outside this
module, cmdAgentSkills (src/init.cts:2259), which destructures
{config, source, degraded} - adding a field does not break it. Its --json IR
now reports degraded:true for a corrupt config, which is the intended fix and
the one observable behavior change.

Closes #1880

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

* fix(#1880): degrade when any config on the path is unusable, not just the last

Two defects found by isolated adversarial review of the first cut.

BLOCKER: the success-path return did not consult configFault. A corrupt ROOT
config whose workstream override happened to parse returned degraded:false /
reason:resolved - the root's settings silently dropped, which is the exact
failure this issue closes, reappearing for any project using workstreams. The
stderr diagnostic fired, so the out-of-band half worked while the in-band half
reported a clean resolve; a --json consumer saw health.

MAJOR: reason was derived from Object.keys(parsed) - the root+workstream MERGE
- so an empty workstream file inheriting a non-empty root reported resolved
despite carrying no settings. Emptiness is now judged on the file actually
read, snapshotted before normalizeLegacyKeys mutates it.

Also: corrects the ConfigResolution JSDoc, which still described the pre-#1880
degraded contract; adds a fast-check property asserting a PRESENT file is
never reported not_configured whatever its bytes (CONTRIBUTING.md parser
rule); and asserts the literal enum values so the provenance lint's
configured_empty/not_configured markers check real assertions rather than
incidental prose in test titles.

Refs #1879

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

* fix(#1880): reject valid JSON that is not a config object at the read seam

The fast-check property added in the previous commit failed on both node
lanes: a config.json containing 0, "str", [], null or true is valid JSON, so
it parsed "ok", then threw downstream in normalizeLegacyKeys, and the outer
catch reported not_configured - a PRESENT file reported as absent, which is
precisely the collapse this issue exists to close. The property asserts a
present file is never not_configured, and it caught it.

_readConfigFile now validates shape, not just parseability (ADR-227: check
the semantic shape at a trust boundary, not merely the type). A non-object
JSON document is an unusable config, reported config_unparseable.

Adds named regression cases for each non-object form alongside the property,
so the class is documented and not only randomly sampled.

Refs #1879

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

* chore(#1880): backfill changeset pr number (pr:0 -> 2688)

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

* test(#2452): record a fetch-time shallow failure instead of crashing

This guard failed CI on ubuntu-24 while passing on ubuntu-22 and
windows-24 for the same commit, and passed on other PRs. Not a flake and not
caused by the change under test - a real fragility in the test.

runnerDiff ran the base fetch OUTSIDE its try and only guarded the diff, so it
assumed the failure mode is always 'fetch succeeds, diff reports no merge
base'. At a shallow boundary that lands short of the merge base, git can
instead fail during the FETCH ('unable to parse commit' - the boundary
commit's parent is not available). Which stage git fails at is version and
transport dependent, so on some runners the error escaped runnerDiff and
crashed the test rather than being recorded as the ok:false the assertions
expect. Both stages mean the same thing for what this guard protects: a
shallow base ref cannot resolve the three-dot diff.

Also drops two assert.match calls against git's stderr prose. 'no merge base'
and 'unable to parse commit' are the same condition reported at different
stages, and CONTRIBUTING prohibits raw text matching on subprocess output.
The typed outcome (ok === false) is the contract; the tests now assert that
plus the presence of a cause.

Found while investigating the red lane on #2688; fixed here per the no-defer
rule rather than filed.

Refs #1879

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-27 00:09:13 -04:00

265 lines
12 KiB
JavaScript

'use strict';
/**
* tests/mutation-workflow-base-ref.test.cjs
*
* Regression tests for the mutation-gate base-ref fetch (issue #2452).
*
* Background: `.github/workflows/mutation.yml` checks out with `fetch-depth: 0`
* (full history) and then re-fetched the base branch with `--depth=1`. That
* shallow re-fetch truncates the base ref's ancestry, so the three-dot diff in
* scripts/mutation-matrix.cjs (`git diff --name-only origin/<base>...HEAD`)
* can no longer compute a merge base and aborts with
* `fatal: origin/next...HEAD: no merge base`, exit 2. The `detect` job then
* fails and the `mutate` shards never run — the 80% mutation-score threshold
* goes UNVERIFIED rather than enforced.
*
* The failure is branch-position dependent, which is why it went unnoticed: a
* branch already level with the base incidentally passes (its merge base IS
* the single fetched commit), while a branch that is BEHIND the base fails.
*
* Test 1 is the contract guard (RED on origin/next, GREEN after the fix).
* Test 2 is a real-git mechanism proof: it reconstructs the runner's ref
* topology in a temp repo and demonstrates that the shallow fetch breaks the
* three-dot diff while a full fetch resolves it — proving the fix is both
* necessary and sufficient rather than asserting on YAML text alone.
*/
const { test, describe } = 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 helpers = require('./helpers.cjs');
const WORKFLOWS_DIR = path.resolve(__dirname, '..', '.github', 'workflows');
/**
* Every workflow whose lint step diffs against the base with the three-dot
* form (`origin/<base>...HEAD`). All of them need the BASE REF's ancestry, so
* none may shallow-fetch it. mutation.yml used --depth=1 and failed outright;
* the other two used --depth=50, shrinking the window further still.
*
* This guard covers the base-ref FETCH only. The shallow *checkout* depth on
* changeset-required.yml / docs-required.yml is a separate, deliberate cost
* control with fail-closed semantics, owned by
* tests/policy-lint-shallow-checkout.test.cjs — do not conflate the two.
*/
const THREE_DOT_WORKFLOWS = [
{ file: 'mutation.yml', consumer: 'scripts/mutation-matrix.cjs' },
{ file: 'changeset-required.yml', consumer: 'scripts/changeset/lint.cjs' },
{ file: 'docs-required.yml', consumer: 'scripts/lint-docs-required.cjs' },
];
// Bounded: git subprocesses in tests must never hang a CI lane.
const GIT_TIMEOUT_MS = 30_000;
function git(cwd, args) {
return execFileSync('git', args, {
cwd,
encoding: 'utf8',
timeout: GIT_TIMEOUT_MS,
stdio: ['ignore', 'pipe', 'pipe'],
});
}
/**
* Extract the `run:` body of the named step from the workflow YAML.
* Deliberately a small hand parser rather than a YAML dep: this asserts on the
* literal command line the runner executes, which is the contract at issue.
*
* Handles both inline (`run: git fetch ...`) and block-scalar (`run: |`) forms;
* a block scalar returns its dedented body so a future refactor to multi-line
* `run:` cannot silently degrade the guard into asserting on the literal "|".
* Comment lines are skipped so a commented-out step of the same name cannot
* shadow the real one.
*/
function runBodyForStep(yaml, stepName) {
const lines = yaml.split(/\r?\n/);
const nameIdx = lines.findIndex(
(l) => !/^\s*#/.test(l) && l.includes(`- name: ${stepName}`),
);
if (nameIdx === -1) return null;
for (let i = nameIdx + 1; i < lines.length; i++) {
const line = lines[i];
// Next step begins -> the step had no run: body.
if (/^\s*- name:/.test(line) && !/^\s*#/.test(line)) return null;
const m = line.match(/^\s*run:\s*(.*)$/);
if (!m) continue;
const inline = m[1].trim();
if (!/^[|>][-+]?$/.test(inline)) return inline;
// Block scalar: collect the indented body until the indentation drops.
const runIndent = line.match(/^(\s*)/)[1].length;
const body = [];
for (let j = i + 1; j < lines.length; j++) {
const bodyLine = lines[j];
if (bodyLine.trim() === '') {
body.push('');
continue;
}
const indent = bodyLine.match(/^(\s*)/)[1].length;
if (indent <= runIndent) break;
body.push(bodyLine.trim());
}
return body.join('\n').trim();
}
return null;
}
describe('#2452 CI gates: base-ref fetch must preserve ancestry', () => {
for (const { file, consumer } of THREE_DOT_WORKFLOWS) {
test(`${file}: "Fetch base ref for diff" does not shallow-fetch the base`, () => {
const yaml = fs.readFileSync(path.join(WORKFLOWS_DIR, file), 'utf8');
const runBody = runBodyForStep(yaml, 'Fetch base ref for diff');
assert.ok(
runBody,
`Expected a "Fetch base ref for diff" step with a run: body in ${file}. ` +
'If the step was renamed, update this test to match — do not delete the guard.',
);
assert.ok(
runBody.startsWith('git fetch origin'),
`Expected the step to fetch the base ref, got: ${runBody}`,
);
assert.ok(
!/--depth[=\s]/.test(runBody),
`${file}: the base-ref fetch must NOT be shallow. A --depth fetch truncates ` +
"the base branch's ancestry, so the three-dot diff in " +
`${consumer} cannot compute a merge base and the job dies with ` +
'`fatal: ...: no merge base` (#2452). Offending command: ' +
runBody,
);
});
}
// How far the base branch advances past the branch point. Chosen so the
// merge base sits OUTSIDE the old --depth=50 window, making the boundary
// between "cushion masks the bug" and "cushion exhausted" directly testable.
const BASE_ADVANCE = 60;
// Base chain is: tip … BASE_ADVANCE commits … branch point. So the branch
// point is the (BASE_ADVANCE + 1)-th commit from the tip — the exact depth
// at which a shallow base fetch first contains a usable merge base.
const MERGE_BASE_DEPTH = BASE_ADVANCE + 1;
test('base-ref fetch depth determines whether the three-dot diff resolves', () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2452-'));
try {
// ---- origin: a base branch that advances past a feature branch --------
const origin = path.join(tmp, 'origin');
fs.mkdirSync(origin);
git(origin, ['init', '--quiet', '--initial-branch=base']);
git(origin, ['config', 'user.email', 'test@example.com']);
git(origin, ['config', 'user.name', 'Test']);
// Ambient commit.gpgsign=true would otherwise break these commits in CI.
git(origin, ['config', 'commit.gpgsign', 'false']);
fs.writeFileSync(path.join(origin, 'seed.txt'), 'seed\n');
git(origin, ['add', '.']);
git(origin, ['commit', '--quiet', '-m', 'seed']);
// Feature branch diverges here — this commit is the merge base.
git(origin, ['checkout', '--quiet', '-b', 'feature']);
fs.writeFileSync(path.join(origin, 'covered.cts'), 'export const x = 1;\n');
git(origin, ['add', '.']);
git(origin, ['commit', '--quiet', '-m', 'feature change']);
// Base then advances, leaving `feature` BEHIND — the failing condition.
git(origin, ['checkout', '--quiet', 'base']);
for (let n = 1; n <= BASE_ADVANCE; n++) {
fs.writeFileSync(path.join(origin, `base-${n}.txt`), `${n}\n`);
git(origin, ['add', '.']);
git(origin, ['commit', '--quiet', '-m', `base advance ${n}`]);
}
// ---- runner: has the PR head, must fetch the base ref separately ------
// Each variant gets its OWN clone, modelling independent workflow runs.
// They must not share a repo: once a shallow fetch writes a .git/shallow
// boundary, a later plain `git fetch` does NOT un-shallow it (that needs
// --unshallow), so repairing in place would test a scenario the workflow
// never encounters.
function runnerDiff(name, baseFetchArgs) {
const dir = path.join(tmp, name);
fs.mkdirSync(dir);
git(dir, ['init', '--quiet']);
git(dir, ['config', 'user.email', 'test@example.com']);
git(dir, ['config', 'user.name', 'Test']);
git(dir, ['config', 'commit.gpgsign', 'false']);
git(dir, ['remote', 'add', 'origin', origin]);
git(dir, ['fetch', '--quiet', 'origin', 'feature']);
git(dir, ['checkout', '--quiet', 'FETCH_HEAD']);
// The FETCH is inside the try, not before it. A base fetch whose shallow
// boundary lands short of the merge base can fail during the FETCH itself
// ("unable to parse commit" — the boundary commit's parent is unavailable)
// rather than succeeding and leaving the DIFF to fail with "no merge base".
// Which of the two git picks is version/transport dependent: this test
// passed on ubuntu-22 and windows-24 and failed on ubuntu-24 for the same
// commit. Both outcomes mean the same thing for what this guard protects —
// a shallow base ref cannot resolve the three-dot diff — so both are
// recorded as ok:false instead of one of them escaping as a crash.
try {
git(dir, ['fetch', '--quiet', 'origin', 'base', ...baseFetchArgs]);
return { ok: true, out: git(dir, ['diff', '--name-only', 'origin/base...HEAD']).trim() };
} catch (err) {
return { ok: false, err: String(err.stderr || err.message) };
}
}
// (a) --depth=1 — mutation.yml's pre-fix command. Always broken.
const depth1 = runnerDiff('runner-depth-1', ['--depth=1']);
assert.equal(
depth1.ok,
false,
'Expected the three-dot diff to FAIL after a --depth=1 base fetch. ' +
'If this stops holding, the #2452 mechanism no longer reproduces and ' +
'this guard needs revisiting.',
);
// Asserting git's exact wording couples this guard to a git version:
// "no merge base" (diff-time) and "unable to parse commit" (fetch-time)
// are the same condition reported at different stages. CONTRIBUTING also
// prohibits raw text matching on subprocess output — the typed outcome
// above (`ok === false`) IS the contract this test exists to pin.
assert.ok(depth1.err.length > 0, 'a failed shallow diff must report a cause');
// (b) BOUNDARY, just below: the merge base is one commit out of reach.
// This is the changeset-required.yml / docs-required.yml --depth=50 case
// generalized — the cushion only ever postponed the same failure.
const below = runnerDiff('runner-depth-below', [`--depth=${MERGE_BASE_DEPTH - 1}`]);
assert.equal(
below.ok,
false,
`Expected FAIL at --depth=${MERGE_BASE_DEPTH - 1} (merge base one commit ` +
'beyond the shallow boundary) — this is why a bounded cushion is not a fix',
);
assert.ok(below.err.length > 0, 'a failed shallow diff must report a cause');
// (c) BOUNDARY, exactly deep enough: the merge base is the last commit in.
const atDepth = runnerDiff('runner-depth-at', [`--depth=${MERGE_BASE_DEPTH}`]);
assert.equal(
atDepth.ok,
true,
`Expected SUCCESS at --depth=${MERGE_BASE_DEPTH} (merge base exactly at the ` +
`shallow boundary), got: ${atDepth.err}`,
);
assert.equal(atDepth.out, 'covered.cts');
// (d) Unbounded — the shipped fix. Correct regardless of how far behind.
const full = runnerDiff('runner-full', []);
assert.equal(full.ok, true, `Expected the full-fetch diff to succeed, got: ${full.err}`);
assert.equal(
full.out,
'covered.cts',
'After a full base fetch the three-dot diff must resolve and report ' +
"exactly the feature branch's changed files",
);
} finally {
helpers.cleanup(tmp);
}
});
});