golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(3407): snapshot old release into gsd-pristine, not new (#87)

Browse Source 
* fix(3407): snapshot old release into gsd-pristine, not new

saveLocalPatches() was wiping gsd-pristine/ then re-populating it from
pristineCtx.packageSrc — the NEW release source tree. For files that
changed between old and new releases, this wrote NEW-release bytes as the
pristine baseline while backup-meta.json recorded OLD-release hashes. The
resulting hash mismatch triggered the #3657 verifier guard on every such
file, causing it to skip the three-way diff baseline and fall back to the
over-broad heuristic — effectively nullifying the #2998 feature for any
file that changed upstream.

Fix: preserve existing gsd-pristine/ entries that are already correct
(sha256 on disk matches originalHash from manifest). These were written
by the previous install with old-release bytes and remain valid. For
files where no correct entry exists, leave gsd-pristine/ absent so the
verifier falls back cleanly to over-broad mode — safe, never false-fails.

OK_PRISTINE_DRIFT_DETECTED (added by #3657) is intentionally kept: it
guards installations that already have drifted pristine from pre-fix runs
and protects against other future stale-pristine scenarios. It is not
removed because its guard is correct; only its trigger frequency drops.

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

* chore: add changeset for #3801

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

* fix(3407): hash-validated regeneration for missing gsd-pristine entries

Codex adversarial review found that the #3407 fix left absent gsd-pristine/
entries permanently absent, causing persistent over-broad verification even
when the file was unchanged between old and new releases.

Add selective regeneration: for entries absent from gsd-pristine/, generate
a candidate via populatePristineDir into a temp dir using new-release source,
then only promote if sha256(candidate) === originalHash. When hashes match,
the file was identical across releases so new-release bytes ARE the correct
old-release pristine. Discard mismatches — over-broad fallback applies.

Add regression test asserting regeneration occurs for unchanged-between-
releases files whose pristine entry was absent.

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

* fix(3407): address review — restore mkdtempSync resilience, tighten tests, fix counter accounting

Addresses sonnet adversarial MAJOR (mkdtempSync outside try/finally caused uncaught
exception on broken /tmp), MIN-01–MIN-05 (misleading prose, counter double-count,
missing stale-pristine test, permissive assertion, vacuous antipattern-hunt), sonnet
MINOR (rmSync EISDIR), NIT (unused import, test count).

F1: move mkdtempSync inside try block with catch/warn for graceful degradation
F2: fix misleading prose — gsd-pristine/ is populated lazily by saveLocalPatches, not
    a separate install-time step
F3: fix counter double-counting — track stalePaths/regeneratedPaths as Sets; removed
    = stale NOT regenerated (non-overlapping counts); update log message accordingly
F4: add stale-pristine recovery test — pre-populates gsd-pristine/ with new-release
    bytes (exact pre-fix bug artifact), asserts absent after fix run
F5: tighten Test 3 assertion from permissive if(exists)/notEqual to strict
    assert.strictEqual(exists, false)
F6: remove vacuous antipattern-hunt describe block (typeof checks only, no behavioral
    coverage) — rationale noted in comment
F7: use fs.rmSync with force:true/recursive:true for EISDIR resilience; only count
    removed after confirming file is actually gone via existsSync
F8: remove unused afterEach from destructured import
F9: test count updated to reflect 4 tests in describe block

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-22 11:24:14 -04:00
committed by GitHub
parent ff1e3d4920
commit 619b5c42e3
3 changed files with 456 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
bin/install.js
View File

@@ -7740,34 +7740,115 @@ function saveLocalPatches(configDir, pristineCtx) {
console.log(' ' + dim + f + reset);
}
// #2998: populate gsd-pristine/ via the install transform pipeline so the
// reapply-patches verifier (#2972) gets a real diff baseline instead of
// falling back to its over-broad "every significant backup line" heuristic.
// #2998 / #3407: maintain gsd-pristine/ as the diff baseline for the
// reapply-patches verifier (#2972).
//
// #3407 root-cause fix: the prior approach (#3004 CR) wiped gsd-pristine/
// and re-populated it from pristineCtx.packageSrc (the NEW release source).
// For files that changed between the old and new release this wrote NEW-
// release bytes as the pristine baseline while backup-meta.json recorded
// OLD-release hashes — a hash mismatch that caused the #3657 verifier guard
// (OK_PRISTINE_DRIFT_DETECTED) to skip the baseline and fall back to over-
// broad mode on every upgrade.
//
// Correct approach: `gsd-pristine/` is populated lazily by saveLocalPatches'
// regenerate branch (not by a separate install-time step); the fix works by
// induction across upgrades — each clean upgrade persists hash-validated
// entries for the next run. During this call we must PRESERVE entries whose
// hash matches originalHash, not overwrite them with new-release bytes.
//
// Per-file decision:
// - sha256(gsd-pristine/X) === originalHash → correct; keep it
// - gsd-pristine/X exists but hash mismatch → stale from a previous
// buggy run (#3407); remove so verifier falls back cleanly
// - gsd-pristine/X absent → attempt hash-validated
// regeneration: generate candidate from new-release source; if
// sha256(candidate) === originalHash the file is identical between
// old and new releases so candidate bytes ARE the old-release pristine
// and can be used; discard otherwise (over-broad fallback)
if (pristineCtx) {
// #3004 CR: wipe any pre-existing pristine content BEFORE populating
// (and again in the catch path). Without this, a previous run's stale
// pristine could be picked up by the verifier as if it were the
// baseline for THIS modified set, causing a misleading three-way diff.
try { fs.rmSync(pristineDir, { recursive: true, force: true }); } catch { /* not present */ }
try {
const written = populatePristineDir({
packageSrc: pristineCtx.packageSrc,
pristineDir,
modified,
runtime: pristineCtx.runtime,
pathPrefix: pristineCtx.pathPrefix,
isGlobal: pristineCtx.isGlobal,
});
if (written > 0) {
console.log(' ' + green + '✓' + reset + ' Populated ' + cyan + 'gsd-pristine/' + reset + ' (' + written + ' file(s)) for three-way merge');
let preserved = 0;
// Track which relPaths had stale pristine entries (hash mismatch) that we
// removed. After the regeneration pass we compute `removed` = stale entries
// that could NOT be recovered (over-broad fallback applies to those only).
const stalePaths = new Set();
// Track which relPaths were successfully regenerated (from either missing or stale).
const regeneratedPaths = new Set();
const missingPaths = [];
for (const relPath of modified) {
const outRef = resolveInstallRelativePath(pristineDir, relPath);
if (!outRef) continue;
const { fullPath: pristinePath } = outRef;
if (fs.existsSync(pristinePath)) {
try {
const onDiskHash = fileHash(pristinePath);
if (onDiskHash === pristineHashes[relPath]) {
preserved++;
continue; // correct old-release bytes already in place — keep them
}
} catch { /* read error — treat as mismatch */ }
// Hash mismatch or read error: stale pristine from a previous buggy
// run (#3407). Remove so verifier falls back to over-broad mode.
try { fs.rmSync(pristinePath, { force: true, recursive: true }); } catch { /* best-effort */ }
// Only count as removed if the file is actually gone post-removal.
if (!fs.existsSync(pristinePath)) {
stalePaths.add(relPath);
}
}
} catch (err) {
// Soft failure: keep the install moving even if the transform pipeline
// throws on an unusual configuration. Wipe the partial pristine so the
// verifier falls back cleanly to its pre-#2998 heuristic instead of
// reading half-populated data (#3004 CR).
try { fs.rmSync(pristineDir, { recursive: true, force: true }); } catch { /* best-effort */ }
console.log(' ' + yellow + 'i' + reset + ' Could not populate gsd-pristine/ (' + (err && err.message ? err.message : 'unknown') + '). Falls back to over-broad verify heuristic.');
// File absent from gsd-pristine/ (or just removed above as stale):
// attempt hash-validated regeneration from new-release source.
missingPaths.push(relPath);
}
// Regenerate missing entries into a temp dir, then validate each hash
// before promoting. Only files whose new-release generated bytes hash to
// originalHash are safe to use — they were unchanged between releases.
if (missingPaths.length > 0) {
let tempPristineDir = null;
try {
tempPristineDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-pristine-regen-'));
populatePristineDir({
packageSrc: pristineCtx.packageSrc,
pristineDir: tempPristineDir,
modified: missingPaths,
runtime: pristineCtx.runtime,
pathPrefix: pristineCtx.pathPrefix,
isGlobal: pristineCtx.isGlobal,
});
for (const relPath of missingPaths) {
const tempRef = resolveInstallRelativePath(tempPristineDir, relPath);
const outRef = resolveInstallRelativePath(pristineDir, relPath);
if (!tempRef || !outRef || !fs.existsSync(tempRef.fullPath)) continue;
try {
const candidateHash = fileHash(tempRef.fullPath);
if (candidateHash !== pristineHashes[relPath]) continue; // new-release differs — discard
fs.mkdirSync(path.dirname(outRef.fullPath), { recursive: true });
fs.copyFileSync(tempRef.fullPath, outRef.fullPath);
regeneratedPaths.add(relPath);
} catch { /* hash or copy error — skip; over-broad fallback applies */ }
}
} catch (err) {
// Match the pre-fix behavior: log a warning and continue (verifier falls back to over-broad mode for missing files).
console.warn(`gsd-pristine regen skipped: ${err.message}`);
} finally {
if (tempPristineDir) {
try { fs.rmSync(tempPristineDir, { recursive: true, force: true }); } catch { /* best-effort */ }
}
}
}
// `regenerated` = total files successfully regenerated (from missing OR stale).
const regenerated = regeneratedPaths.size;
// `removed` = stale entries that were deleted and NOT subsequently regenerated.
// Entries that were stale-deleted but then successfully regenerated are counted
// only in `regenerated` — the counts are non-overlapping.
const removed = [...stalePaths].filter(p => !regeneratedPaths.has(p)).length;
if (preserved > 0) {
console.log(' ' + green + '✓' + reset + ' Preserved ' + cyan + 'gsd-pristine/' + reset + ' (' + preserved + ' file(s)) for three-way merge');
}
if (regenerated > 0) {
console.log(' ' + green + '✓' + reset + ' Regenerated ' + cyan + 'gsd-pristine/' + reset + ' (' + regenerated + ' file(s)) via hash-validated new-release source');
}
if (removed > 0) {
console.log(' ' + yellow + 'i' + reset + ' Removed ' + removed + ' stale gsd-pristine/ snapshot(s); regenerated ' + regenerated + ' of those — falls back to over-broad verify heuristic for the rest');
}
}
}