Files
msd-core/tests/bug-2969-verify-reapply-patches.test.cjs
Tom Boucher a7abc6df2f fix(3657): skip false-fail when pristine hash drifts after GSD update (#3767)
* test(3657): RED+shape-lock for verify-reapply-patches pristine-drift

Adds tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs:
- Core regression: exits 0 with reason=OK_PRISTINE_DRIFT_DETECTED when
  on-disk gsd-pristine/ hash does not match backup-meta.json.pristine_hashes
- Counter-tests: real FAIL_USER_LINES_MISSING still caught when hashes match;
  over-broad mode unchanged when backup-meta.json is absent; clean run
  reports 0 failures when everything matches
- Multi-file: drift + real-failure handled independently per file

Updates tests/bug-2969-verify-reapply-patches.test.cjs REASON shape-lock to
include OK_PRISTINE_DRIFT_DETECTED (added by the #3657 fix).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(3657): verify-reapply-patches skips pristine when hash drifts

When gsd-pristine/ is refreshed to a newer GSD version after a backup is
captured, the on-disk pristine's SHA-256 no longer matches the hash recorded
in backup-meta.json.pristine_hashes.  Using the wrong-version pristine as the
diff baseline inverts the delta: every line the upstream removed between the
two versions appears as a "user-added line that must survive", producing
spurious FAIL_USER_LINES_MISSING false positives (Bug #3657).

Fix:
- Add sha256() and readPristineHashes() helpers to verify-reapply-patches.cjs
- In verifyFile(), when a pristine_hashes entry exists for the file, compare
  the on-disk pristine's SHA-256 against it before accepting the baseline
- On hash mismatch, return immediately with status=ok and the new
  REASON.OK_PRISTINE_DRIFT_DETECTED code, skipping the diff rather than
  false-failing
- When no pristine_hashes entry exists (older installer / absent backup-meta),
  fall through to the pre-fix behaviour (use on-disk pristine as-is)
- Export sha256, readPristineHashes, and OK_PRISTINE_DRIFT_DETECTED

New REASON code OK_PRISTINE_DRIFT_DETECTED is added to the frozen enum.
Exit code contract is unchanged: 0 for gate pass (including skipped-due-drift
files), 1 for real user-content failures, 2 for structural errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(3657): update changeset to reference PR #3767

* fix(3657): surface drifted_files in verify-reapply-patches JSON report

Extend the top-level JSON report shape with two additive fields:
  - `drifted: N`  — count of files skipped due to pristine-snapshot drift
  - `drifted_files: [...]`  — relative paths of those files

Per-file shape is unchanged (status:'ok' + reason:OK_PRISTINE_DRIFT_DETECTED)
for backward compat. Drift still exits 0; `failures` count is unaffected.

This gives workflow Step 5a structured data to gate on so drifted files are
no longer silently treated as a full PASS (codex adversarial-review finding 1).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(3657): reapply-patches workflow halts on drifted files instead of silent pass

Insert a "Step 5a: drift check" block between the exit-code check and the
failures check in workflows/reapply-patches.md Step 5a.  The new block:

  1. Parses `drifted` + `drifted_files` from the JSON report (added in the
     companion prod-code commit).
  2. When DRIFTED_COUNT > 0, emits a formatted HALT message naming each
     drifted file and instructs the user to re-baseline before re-running.
  3. Sets DRIFT_DETECTED=true and exits non-zero so subsequent steps cannot
     execute while drift is unresolved.

Drift is a distinct third state: it is not a failure (no missing user lines
were proven) but it is also not a clean pass (the diff was skipped entirely).
Existing pass/fail logic for VERIFY_STATUS and failures count is unchanged.

Closes the silent-skip gap identified in codex adversarial-review finding 1.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(3657): assert drifted_files report shape + workflow Step 5a drift check

Finding 1 (BLOCKER) — three new tests in bug-3657 test file:
  - Single drifted file: drifted=1, drifted_files contains the file path,
    failures=0, per-file shape unchanged (backward compat).
  - Multi-file drift: drifted=2, drifted_files lists both paths, clean file
    absent from array.
  - No-drift baseline: drifted=0, drifted_files=[] always present in output.

Finding 2 (WARNING) — structural test on workflow source:
  - Asserts Step 5a contains "Step 5a: drift check" heading.
  - Asserts DRIFTED_COUNT, drifted_files, and DRIFT_DETECTED are referenced
    (confirming the gate exists and uses the structured report fields).
  - Asserts drift-check block appears before VERIFY_STATUS check (exit-code
    is 0 for drift, so the drift check must precede the non-zero gate).

Also updates bug-2969 shape-lock to include the two new additive fields
(drifted, drifted_files) per the contract change in the prod-code commit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(3657): address pr-review-toolkit + codex review + CI failures

- lint-tests: add // allow-test-rule: source-text-is-the-product at file
  top of bug-3657 test file; the inline comment at line 477 was a prose
  sentence, not a file-level annotation, so the lint scanner did not
  recognise it as the bypass token
- Windows test failures (4 subtests): normalize relPath to forward slashes
  before pristineHashes key lookup in verifyFile(); on Windows path.join
  produces backslash-separated relPath values but backup-meta.json stores
  keys with forward slashes, causing the hash lookup to silently return
  undefined, falling through to use-as-is mode and producing the same
  false FAIL_USER_LINES_MISSING that the fix was meant to prevent

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(tests): bump verify allowlist ceiling 9→10 for bug-3657 test file

Adding tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs in
the previous commit pushed the verify module from 9 to 10 test files.
The lint-test-file-count gate (added in #6313baad on main) enforces that
modules cannot exceed their allowlist ceiling, so all 6 test platforms
plus lint-tests and coverage failed with:

  FAIL_EXCEEDS_ALLOWLIST: verify count=10 ceiling=9

The fix is to raise the ceiling from 9 to 10 and set issue=3767.
This file does not exist on this branch yet (introduced on main after
the branch diverged) so we add it here. The merged CI state will see
the bumped ceiling and pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-20 23:13:25 -04:00

214 lines
8.9 KiB
JavaScript

'use strict';
process.env.GSD_TEST_MODE = '1';
/**
* Bug #2969: /gsd-reapply-patches Step 5 hunk verification gate reports
* success on lost content because the LLM-driven workflow fills in
* "verified: yes" without actually checking content presence.
*
* Fix: deterministic verifier script (scripts/verify-reapply-patches.cjs)
* that the workflow calls.
*
* Per the repo's no-source-grep testing standard (CONTRIBUTING.md):
* tests must assert on TYPED structured fields — not regex/substring
* matching against script output, formatter prose, or file content.
*
* The script's --json mode emits a structured report whose `reason`
* field is a stable enum (exposed as REASON), and whose `missing` field
* is an array of typed strings (exact set membership, not substring).
* Every assertion below is a deepEqual / equal / Array.includes against
* those typed fields. Zero regex, zero String#includes on text.
*/
const { test, describe, before, after } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const cp = require('node:child_process');
const ROOT = path.join(__dirname, '..');
// Script lives at get-shit-done/bin/ so the installer ships it under
// `${GSD_HOME}/get-shit-done/bin/` (issue #2994). The top-level scripts/
// directory is not copied to user installs.
const SCRIPT = path.join(ROOT, 'get-shit-done', 'bin', 'verify-reapply-patches.cjs');
const { REASON } = require(SCRIPT);
let tmpRoot;
let patchesDir;
let configDir;
let pristineDir;
function writeFile(absPath, content) {
fs.mkdirSync(path.dirname(absPath), { recursive: true });
fs.writeFileSync(absPath, content);
}
function resetFixture({ withPristine = true } = {}) {
for (const dir of [patchesDir, configDir, pristineDir]) {
fs.rmSync(dir, { recursive: true, force: true });
}
fs.mkdirSync(patchesDir);
fs.mkdirSync(configDir);
if (withPristine) fs.mkdirSync(pristineDir);
}
/** Runs the verifier with --json. Returns parsed structured report. */
function runVerifier({ includePristine = true } = {}) {
const args = [
SCRIPT,
'--patches-dir', patchesDir,
'--config-dir', configDir,
...(includePristine ? ['--pristine-dir', pristineDir] : []),
'--json',
];
const r = cp.spawnSync(process.execPath, args, { encoding: 'utf8' });
return {
status: r.status,
report: r.stdout && r.stdout.length ? JSON.parse(r.stdout) : null,
};
}
before(() => {
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2969-'));
patchesDir = path.join(tmpRoot, 'patches');
configDir = path.join(tmpRoot, 'installed');
pristineDir = path.join(tmpRoot, 'pristine');
resetFixture();
});
after(() => {
fs.rmSync(tmpRoot, { recursive: true, force: true });
});
describe('Bug #2969: deterministic Step 5 verification gate', () => {
test('REASON enum exposes the documented set of stable codes', () => {
// Locks the public diagnostic surface — adding a code requires updating
// this assertion, removing one breaks consumers that switch on the enum.
// Bug #3657 added OK_PRISTINE_DRIFT_DETECTED.
assert.deepEqual(
Object.keys(REASON).sort(),
[
'FAIL_INSTALLED_MISSING',
'FAIL_INSTALLED_NOT_REGULAR_FILE',
'FAIL_READ_ERROR',
'FAIL_USER_LINES_MISSING',
'OK_NO_SIGNIFICANT_BACKUP_LINES',
'OK_NO_USER_LINES_VS_PRISTINE',
'OK_PRISTINE_DRIFT_DETECTED',
],
);
});
test('exits 0 with status=ok when every user-added line is present in the merged file', () => {
resetFixture();
const pristine = 'line one of stock content here\nline two of stock content here\nline three of stock content here\n';
const userAdded = 'a custom line the user added for behavior X\nanother substantial line that the user inserted\n';
writeFile(path.join(pristineDir, 'skills', 'foo', 'SKILL.md'), pristine);
writeFile(path.join(patchesDir, 'skills', 'foo', 'SKILL.md'), pristine + userAdded);
writeFile(path.join(configDir, 'skills', 'foo', 'SKILL.md'), pristine + userAdded);
const { status, report } = runVerifier();
assert.equal(status, 0);
assert.equal(report.failures, 0);
assert.equal(report.checked, 1);
assert.equal(report.results[0].status, 'ok');
assert.deepEqual(report.results[0].missing, []);
});
test('reason=FAIL_USER_LINES_MISSING with the exact dropped line in .missing[]', () => {
resetFixture();
const pristine = 'first stock line in the original file here\nsecond stock line in the original file here\n';
const lostLine = 'this is the visual companion block that must survive';
writeFile(path.join(pristineDir, 'skills', 'discuss-phase', 'SKILL.md'), pristine);
writeFile(path.join(patchesDir, 'skills', 'discuss-phase', 'SKILL.md'), `${pristine}${lostLine}\n`);
writeFile(path.join(configDir, 'skills', 'discuss-phase', 'SKILL.md'), pristine);
const { status, report } = runVerifier();
assert.equal(status, 1);
assert.equal(report.failures, 1);
const r0 = report.results[0];
// Normalize separators: on Windows the SUT emits 'skills\discuss-phase\SKILL.md'.
assert.equal(r0.file.replace(/\\/g, '/'), 'skills/discuss-phase/SKILL.md');
assert.equal(r0.status, 'fail');
assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING);
assert.ok(
r0.missing.includes(lostLine),
`dropped line should be in .missing[]; got ${JSON.stringify(r0.missing)}`,
);
});
test('reason=FAIL_INSTALLED_NOT_REGULAR_FILE when installed path is a directory', () => {
resetFixture();
writeFile(path.join(pristineDir, 'a.md'), 'pristine line of substantial content here\n');
writeFile(path.join(patchesDir, 'a.md'), 'pristine line of substantial content here\nuser added line that is substantial\n');
fs.mkdirSync(path.join(configDir, 'a.md')); // EISDIR trap
const { status, report } = runVerifier();
assert.equal(status, 1);
assert.equal(report.results[0].status, 'fail');
assert.equal(report.results[0].reason, REASON.FAIL_INSTALLED_NOT_REGULAR_FILE);
});
test('reason=FAIL_INSTALLED_MISSING when the merged file has been deleted', () => {
resetFixture();
const pristine = 'stock line one with substantial content for the test\n';
writeFile(path.join(pristineDir, 'workflow.md'), pristine);
writeFile(path.join(patchesDir, 'workflow.md'), `${pristine}user line that should survive but does not\n`);
// configDir intentionally missing the file.
const { status, report } = runVerifier();
assert.equal(status, 1);
assert.equal(report.results[0].status, 'fail');
assert.equal(report.results[0].reason, REASON.FAIL_INSTALLED_MISSING);
});
test('--json report has the documented shape: { checked, failures, results: [{ file, status, missing, reason }] }', () => {
resetFixture();
const pristine = 'pristine line that is sufficiently long to be significant\n';
const userAdded = 'extra line the user wrote for their workflow customisation';
writeFile(path.join(pristineDir, 'a.md'), pristine);
writeFile(path.join(patchesDir, 'a.md'), `${pristine}${userAdded}\n`);
writeFile(path.join(configDir, 'a.md'), pristine);
const { status, report } = runVerifier();
assert.equal(status, 1);
// Bug #3657 (Finding 1): drifted + drifted_files are additive fields added to surface
// pristine-drift skips distinctly from failures. Shape-lock updated to include them.
assert.deepEqual(Object.keys(report).sort(), ['checked', 'drifted', 'drifted_files', 'failures', 'results']);
const r0 = report.results[0];
assert.deepEqual(Object.keys(r0).sort(), ['file', 'missing', 'reason', 'status']);
assert.equal(typeof r0.file, 'string');
assert.equal(typeof r0.status, 'string');
assert.equal(typeof r0.reason, 'string');
assert.ok(Array.isArray(r0.missing));
});
test('ignores backup-meta.json — it is metadata, not a patched file', () => {
resetFixture();
writeFile(path.join(patchesDir, 'backup-meta.json'), JSON.stringify({ files: [] }));
const { status, report } = runVerifier();
assert.equal(status, 0);
assert.equal(report.checked, 0);
assert.equal(report.failures, 0);
assert.deepEqual(report.results, []);
});
test('without --pristine-dir, treats every significant backup line as required (safe over-broad fallback)', () => {
resetFixture({ withPristine: false });
const presentLine = 'this is a substantial line of user content here';
const droppedLine = 'another substantial line that should survive';
writeFile(path.join(patchesDir, 'b.md'), `${presentLine}\n${droppedLine}\n`);
writeFile(path.join(configDir, 'b.md'), `${presentLine}\n`);
const { status, report } = runVerifier({ includePristine: false });
assert.equal(status, 1);
assert.equal(report.results[0].reason, REASON.FAIL_USER_LINES_MISSING);
assert.ok(report.results[0].missing.includes(droppedLine));
assert.ok(!report.results[0].missing.includes(presentLine));
});
});