Files
msd-core/get-shit-done/bin/verify-reapply-patches.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

337 lines
13 KiB
JavaScript
Executable File

#!/usr/bin/env node
'use strict';
/**
* Deterministic verifier for the /gsd-reapply-patches Step 5 "Hunk Verification
* Gate". For each backed-up patch file, asserts that the user's added lines
* (computed from a real diff against the pristine baseline, not from the
* LLM's prose summary) survive into the merged output.
*
* Usage:
* node scripts/verify-reapply-patches.cjs \
* --patches-dir <path> \ # gsd-local-patches/
* --config-dir <path> \ # ~/.claude (or runtime equivalent)
* [--pristine-dir <path>] # gsd-pristine/; if absent, falls back to
* # treating every significant backup line as
* # required (over-broad but safe for #2969:
* # false-positive halts beat silent successes
* # on lost content)
* [--json] # emit JSON report instead of human text
*
* Exit codes:
* 0 — every user-added line is present in the merged file (gate passes)
* 1 — at least one missing line in at least one file (gate fails)
* 2 — usage / structural error (e.g. patches dir missing)
*
* Bug #2969: the Step 5 gate previously trusted Claude's free-text "verified:
* yes/no" reporting per hunk. The LLM was filling in `yes` even when content
* had been silently dropped. Moving the check to a deterministic script is the
* durability fix.
*/
const fs = require('node:fs');
const path = require('node:path');
const crypto = require('node:crypto');
const SIGNIFICANT_MIN_CHARS = 12;
function parseArgs(argv) {
const opts = { patchesDir: null, configDir: null, pristineDir: null, json: false };
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === '--patches-dir') opts.patchesDir = argv[++i];
else if (arg === '--config-dir') opts.configDir = argv[++i];
else if (arg === '--pristine-dir') opts.pristineDir = argv[++i];
else if (arg === '--json') opts.json = true;
else if (arg === '--help' || arg === '-h') {
process.stdout.write(
'usage: verify-reapply-patches.cjs --patches-dir <path> --config-dir <path> [--pristine-dir <path>] [--json]\n',
);
process.exit(0);
} else {
process.stderr.write(`unknown argument: ${arg}\n`);
process.exit(2);
}
}
return opts;
}
function isSignificantLine(line) {
const trimmed = line.trim();
if (trimmed.length < SIGNIFICANT_MIN_CHARS) return false;
// Pure punctuation / closing brackets carry too little structural info to
// reliably distinguish a survived hunk from incidental similarity.
if (/^[\s})\];,]+$/.test(trimmed)) return false;
// Generic decorative comments like `// ----` similarly fail the test.
if (/^[\s\-=#*/]+$/.test(trimmed)) return false;
return true;
}
/**
* Compute the SHA-256 hex digest of a string (UTF-8 encoded).
*/
function sha256(content) {
return crypto.createHash('sha256').update(content, 'utf8').digest('hex');
}
/**
* Read the `pristine_hashes` map from backup-meta.json in the patches dir.
* Returns an empty object if backup-meta.json is absent, unreadable, or has no
* `pristine_hashes` field — callers must treat an empty map as "no recorded
* hash for any file" (no hash-validation possible, not an error).
*/
function readPristineHashes(patchesDir) {
const metaPath = path.join(patchesDir, 'backup-meta.json');
try {
const raw = fs.readFileSync(metaPath, 'utf8');
const parsed = JSON.parse(raw);
if (parsed && typeof parsed.pristine_hashes === 'object' && parsed.pristine_hashes !== null) {
return parsed.pristine_hashes;
}
} catch {
// absent or unreadable — not an error, just no recorded hashes
}
return {};
}
/**
* Walk a directory, returning every file's path relative to the root.
*/
function walk(rootDir, relPrefix = '') {
const out = [];
if (!fs.existsSync(rootDir)) return out;
for (const entry of fs.readdirSync(rootDir, { withFileTypes: true })) {
const rel = relPrefix ? path.join(relPrefix, entry.name) : entry.name;
const abs = path.join(rootDir, entry.name);
if (entry.isDirectory()) {
out.push(...walk(abs, rel));
} else if (entry.isFile()) {
out.push(rel);
}
}
return out;
}
/**
* Compute the set of "user-added" lines: lines present in the backup but
* absent from the pristine baseline. If no pristine is provided, falls back
* to using every significant line in the backup (over-broad but safe — favours
* false-positive failures over silent successes, which is the right side to
* err on for #2969).
*/
function computeUserAddedLines(backupContent, pristineContent) {
const backupLines = backupContent.split(/\r?\n/);
if (!pristineContent) {
return backupLines.filter(isSignificantLine);
}
const pristineSet = new Set(pristineContent.split(/\r?\n/));
return backupLines.filter((line) => isSignificantLine(line) && !pristineSet.has(line));
}
/**
* Stable reason codes for the per-file result. Tests assert via
* `assert.equal(result.reason, REASON.X)` rather than regex-matching prose,
* so the diagnostic surface is a typed enum, not free text.
*
* Adding a new reason requires updating the REASON map AND the tests'
* shape assertion that locks the documented set of codes.
*/
const REASON = Object.freeze({
OK_NO_USER_LINES_VS_PRISTINE: 'ok_no_user_lines_vs_pristine',
OK_NO_SIGNIFICANT_BACKUP_LINES: 'ok_no_significant_backup_lines',
// Bug #3657: the on-disk gsd-pristine/ file's SHA-256 does not match the
// hash recorded in backup-meta.json.pristine_hashes. This means the
// pristine snapshot was refreshed to a newer GSD version after the backup
// was captured. Using the wrong-version pristine as the diff baseline would
// invert the delta (upstream removals appear as "missing user lines").
// The verifier skips this file rather than false-failing it. A separate
// re-anchor step (or the git-aware fallback in the workflow) is needed to
// resolve this file; the guard here ensures the gate does not report spurious
// failures in the meantime.
OK_PRISTINE_DRIFT_DETECTED: 'ok_pristine_drift_detected',
FAIL_INSTALLED_MISSING: 'fail_installed_missing',
FAIL_INSTALLED_NOT_REGULAR_FILE: 'fail_installed_not_regular_file',
FAIL_READ_ERROR: 'fail_read_error',
FAIL_USER_LINES_MISSING: 'fail_user_lines_missing',
});
function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashes }) {
const backupPath = path.join(patchesDir, relPath);
const installedPath = path.join(configDir, relPath);
const result = { file: relPath, status: 'ok', missing: [], reason: null };
if (!fs.existsSync(backupPath) || !fs.statSync(backupPath).isFile()) {
return result; // walked entry no longer exists — non-fatal
}
// Installed path checks: must exist, must be a regular file, must be
// readable. Anything else is a fail-with-diagnostic, not a crash that
// aborts the whole gate run and drops structured output.
let installedStat;
try {
installedStat = fs.statSync(installedPath);
} catch {
result.status = 'fail';
result.reason = REASON.FAIL_INSTALLED_MISSING;
return result;
}
if (!installedStat.isFile()) {
result.status = 'fail';
result.reason = REASON.FAIL_INSTALLED_NOT_REGULAR_FILE;
return result;
}
let backupContent;
let installedContent;
try {
backupContent = fs.readFileSync(backupPath, 'utf8');
installedContent = fs.readFileSync(installedPath, 'utf8');
} catch {
result.status = 'fail';
result.reason = REASON.FAIL_READ_ERROR;
return result;
}
let pristineContent = null;
if (pristineDir) {
const pristinePath = path.join(pristineDir, relPath);
try {
const stat = fs.statSync(pristinePath);
if (stat.isFile()) {
const candidate = fs.readFileSync(pristinePath, 'utf8');
// Bug #3657: if backup-meta.json recorded a pristine_hash for this
// file, validate that the on-disk pristine matches it. A mismatch
// means the installer refreshed gsd-pristine/ to a newer GSD version
// after the backup was captured. Using the wrong-version pristine as
// the diff baseline inverts the delta: upstream removals appear as
// "user-added lines that must survive", causing FAIL_USER_LINES_MISSING
// false positives. When the hash is stale, skip the pristine and fall
// through to over-broad mode (every significant backup line is checked).
// Over-broad mode never false-fails for a different reason because all
// backup lines that are genuinely user-added will still be present in a
// correctly merged install.
// Normalize to forward slashes so the key lookup matches on Windows
// where path.join produces backslash-separated relPath values but
// backup-meta.json stores keys written with forward slashes.
const hashKey = relPath.replace(/\\/g, '/');
const recordedHash = pristineHashes && pristineHashes[hashKey];
if (recordedHash) {
if (sha256(candidate) === recordedHash) {
// Hash matches: the on-disk pristine is the correct baseline.
pristineContent = candidate;
} else {
// Hash mismatch: the on-disk gsd-pristine/ was refreshed to a newer
// GSD version after the backup was captured. Using it as the diff
// baseline would invert the delta and produce false FAIL_USER_LINES_MISSING
// reports (Bug #3657). Report the file as ok with a diagnostic code
// so the gate does not false-fail; a re-anchor or git-aware baseline
// step is required to verify this file correctly.
result.reason = REASON.OK_PRISTINE_DRIFT_DETECTED;
return result;
}
} else {
// No recorded hash for this file (older installer or absent
// backup-meta) — use the on-disk pristine as-is (pre-fix behaviour).
pristineContent = candidate;
}
}
} catch {
// Pristine missing or unreadable — fall through to over-broad mode.
}
}
const userAdded = computeUserAddedLines(backupContent, pristineContent);
if (userAdded.length === 0) {
// Backup and pristine match exactly (or no significant content) — nothing
// to verify but also nothing to lose. Report as ok with diagnostic code.
result.reason = pristineContent
? REASON.OK_NO_USER_LINES_VS_PRISTINE
: REASON.OK_NO_SIGNIFICANT_BACKUP_LINES;
return result;
}
for (const line of userAdded) {
if (!installedContent.includes(line)) {
result.missing.push(line.trim());
}
}
if (result.missing.length > 0) {
result.status = 'fail';
result.reason = REASON.FAIL_USER_LINES_MISSING;
}
return result;
}
function main() {
const opts = parseArgs(process.argv.slice(2));
if (!opts.patchesDir || !opts.configDir) {
process.stderr.write('--patches-dir and --config-dir are required\n');
process.exit(2);
}
if (!fs.existsSync(opts.patchesDir)) {
process.stderr.write(`patches dir not found: ${opts.patchesDir}\n`);
process.exit(2);
}
if (!fs.existsSync(opts.configDir)) {
process.stderr.write(`config dir not found: ${opts.configDir}\n`);
process.exit(2);
}
const files = walk(opts.patchesDir).filter((f) => !f.endsWith('backup-meta.json'));
// Bug #3657: read pristine_hashes from backup-meta.json once and share
// across all per-file verifications so each can detect drift independently.
const pristineHashes = readPristineHashes(opts.patchesDir);
const results = files.map((relPath) =>
verifyFile({
relPath,
patchesDir: opts.patchesDir,
configDir: opts.configDir,
pristineDir: opts.pristineDir,
pristineHashes,
}),
);
const failures = results.filter((r) => r.status === 'fail');
// Bug #3657 (Finding 1): aggregate drifted files into top-level report fields
// so workflow Step 5a can gate on drift distinctly from failures. Drift is
// NOT a failure (exit code stays 0) but the workflow now has structured data
// to decide whether to proceed. Per-file shape is unchanged: each drifted
// result still has status:'ok' + reason:OK_PRISTINE_DRIFT_DETECTED for
// backward compat. The new top-level fields are purely additive.
const driftedResults = results.filter((r) => r.reason === REASON.OK_PRISTINE_DRIFT_DETECTED);
const drifted = driftedResults.length;
const drifted_files = driftedResults.map((r) => r.file);
if (opts.json) {
process.stdout.write(
JSON.stringify({ checked: results.length, failures: failures.length, drifted, drifted_files, results }, null, 2) + '\n',
);
} else {
process.stdout.write(`# Hunk Verification Gate (#2969)\n\n`);
process.stdout.write(`Checked: ${results.length} file(s)\n`);
process.stdout.write(`Failures: ${failures.length}\n\n`);
if (failures.length > 0) {
process.stdout.write(`## Files with missing user-added content\n\n`);
for (const r of failures) {
process.stdout.write(`- ${r.file}\n`);
if (r.reason) process.stdout.write(` reason: ${r.reason}\n`);
for (const line of r.missing.slice(0, 5)) {
process.stdout.write(` missing: ${line}\n`);
}
if (r.missing.length > 5) {
process.stdout.write(` …and ${r.missing.length - 5} more line(s)\n`);
}
}
}
}
process.exit(failures.length > 0 ? 1 : 0);
}
if (require.main === module) {
main();
}
module.exports = { computeUserAddedLines, isSignificantLine, verifyFile, walk, REASON, readPristineHashes, sha256 };