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

fix(#2998): populate gsd-pristine/ from install transform pipeline so verifier has a real baseline (#3004)

Browse Source 
* fix(#2998): populate gsd-pristine/ from install transform pipeline so verifier has a real baseline

saveLocalPatches declared a pristineDir variable and JSDoc'd 'saves
pristine copies to gsd-pristine/' but no code ever wrote there. Effect:
/gsd-reapply-patches Step 5 verifier (#2972) silently fell back to its
over-broad heuristic ('every significant backup line') -- exactly the
silent-success-on-lost-content failure mode #2969 was designed to
prevent.

Fix: new populatePristineDir({...}) helper runs copyWithPathReplacement
(the install transform pipeline) into a tmp staging dir, then copies out
only the modified-file paths into gsd-pristine/. saveLocalPatches now
accepts a pristineCtx and calls the helper when local patches are
detected. Soft-fails on transform errors (logs warning, continues with
empty pristine -- no worse than pre-fix).

Pristine reflects the about-to-install version's content, which is the
right baseline for 'what would survive without the user's modifications'.

Tests: bug-2998-pristine-dir-populated.test.cjs asserts the helper is
exported, no-ops on empty input, writes one pristine file per source-
existing path, skips ghost paths, and produces deterministic output
(byte-identical across runs -- the property pristine_hashes depends on).

Closes #2998

* chore(#2998): add changeset fragment for PR #3004

* fix(#2998): expand pristine to all manifest install roots; clear stale pristine on populate (CR)

CodeRabbit on PR #3004 caught two issues:

1. populatePristineDir only staged packageSrc/get-shit-done/ but
   manifest.files records edits under several install roots (commands/,
   agents/, hooks/, skills/, root files like .clinerules). Modified
   paths outside get-shit-done/ were silently skipped, leaving the
   verifier with no baseline for those edits. Fixed by computing the
   set of top-level dirs from the modified set and staging each one
   that exists in source. Root-level files (no slash) bypass the
   transform pipeline and are copied directly.

2. populatePristineDir did not wipe pre-existing gsd-pristine/ before
   populating. A previous run's stale pristine could survive into the
   current run's diff baseline. Now wipe before populate AND in the
   catch path so soft-failures don't leave half-populated data on disk.

Tests: bug-2998-pristine-dir-populated.test.cjs grows by 2 cases:
- agents/ paths are staged and copied (was silently skipped pre-fix)
- mixed get-shit-done/ + agents/ in same modified list both stage
This commit is contained in:
Tom Boucher
2026-05-01 21:14:14 -04:00
committed by GitHub
parent c2ada7e799
commit 9f09246f3b
4 changed files with 343 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

145
bin/install.js
View File

@@ -6798,13 +6798,98 @@ function writeManifest(configDir, runtime = 'claude', options = {}) {
return manifest;
}
/**
* Populate gsd-pristine/ with the transformed pristine versions of every
* `modified` file, derived from the current package's source tree by
* running the install transform pipeline (`copyWithPathReplacement`)
* into a tmp directory, then copying out only the relevant paths.
*
* Pristine semantically represents "what the install would write to
* configDir/<relPath> if the user had not modified it." This is what the
* /gsd-reapply-patches Step 5 verifier (#2972) uses as the diff base
* for "user-added lines" — lines in the user's backup that are NOT in
* the pristine baseline. Without this dir, the verifier degrades to its
* over-broad fallback ("every significant backup line"), exactly the
* silent-success-on-lost-content failure mode #2969 was designed to
* prevent (#2998).
*
* Implementation note: we run the FULL transform pipeline against a tmp
* staging dir (one-time, only when modified.length > 0), then copy out
* just the modified paths. This re-uses the existing transform code
* exactly — pristine is byte-identical to what `copyWithPathReplacement`
* would have written under normal install. Cost: one extra full transform
* pass per install where local patches were detected; acceptable.
*/
function populatePristineDir({ packageSrc, pristineDir, modified, runtime, pathPrefix, isGlobal }) {
if (!modified || modified.length === 0) return 0;
// Modified paths come from manifest.files which can live under several
// install roots: get-shit-done/, commands/gsd/, command/, skills/, agents/,
// hooks/, plus runtime-specific root files (#3004 CR). Stage every
// top-level dir that actually contains a modified path; root-level files
// are copied directly without the transform pipeline (they don't need
// path replacement).
const stageRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-pristine-stage-'));
let written = 0;
try {
const topLevels = new Set();
for (const relPath of modified) {
const norm = relPath.replace(/\\/g, '/');
const slash = norm.indexOf('/');
topLevels.add(slash === -1 ? '' : norm.slice(0, slash));
}
for (const top of topLevels) {
if (top === '') {
// Root-level files — copy directly from package source. The transform
// pipeline is directory-oriented; root files don't need path-prefix
// substitution (they're not markdown content with embedded paths).
for (const relPath of modified) {
const norm = relPath.replace(/\\/g, '/');
if (norm.includes('/')) continue;
const src = path.join(packageSrc, relPath);
if (!fs.existsSync(src)) continue;
const stagedFile = path.join(stageRoot, relPath);
fs.mkdirSync(path.dirname(stagedFile), { recursive: true });
fs.copyFileSync(src, stagedFile);
}
continue;
}
const srcDir = path.join(packageSrc, top);
const stageDir = path.join(stageRoot, top);
if (!fs.existsSync(srcDir)) continue;
copyWithPathReplacement(srcDir, stageDir, pathPrefix, runtime, false, isGlobal);
}
for (const relPath of modified) {
// Only populate pristine for paths we successfully staged. If a path's
// source dir does not exist (obsolete manifest entry), skip silently
// rather than corrupting pristine with stale data.
const stagedPath = path.join(stageRoot, relPath);
if (!fs.existsSync(stagedPath)) continue;
const out = path.join(pristineDir, relPath);
fs.mkdirSync(path.dirname(out), { recursive: true });
fs.copyFileSync(stagedPath, out);
written++;
}
} finally {
try { fs.rmSync(stageRoot, { recursive: true, force: true }); } catch { /* best-effort cleanup */ }
}
return written;
}
/**
* Detect user-modified GSD files by comparing against install manifest.
* Backs up modified files to gsd-local-patches/ for reapply after update.
* Also saves pristine copies (from manifest) to gsd-pristine/ to enable
* three-way merge during reapply-patches (pristine vs user vs new).
*
* The optional `pristineCtx` parameter (set by the install entry point)
* carries the source package root, runtime, pathPrefix, and isGlobal
* needed to populate gsd-pristine/. If omitted (legacy callers), pristine
* stays empty — the verifier falls back to its over-broad heuristic, same
* behavior as before #2998.
*/
function saveLocalPatches(configDir) {
function saveLocalPatches(configDir, pristineCtx) {
const manifestPath = path.join(configDir, MANIFEST_NAME);
if (!fs.existsSync(manifestPath)) return [];
@@ -6836,16 +6921,12 @@ function saveLocalPatches(configDir) {
}
}
// Save pristine copies of modified files from the CURRENT install (before wipe)
// These represent the original GSD distribution files that the user then modified.
// The reapply-patches workflow uses these for three-way merge:
// pristine (original) → user's version (what they changed) → new version (after update)
// Save pristine copies of modified files from the CURRENT install (before wipe).
// Pristine semantically represents "what the install would write to configDir
// if the user had not modified it" — used by /gsd-reapply-patches Step 5
// (#2972) as the diff baseline for the user-added-lines computation. Without
// this dir the verifier degrades to its over-broad fallback heuristic (#2998).
if (modified.length > 0) {
// We need the pristine originals, but the current files on disk are user-modified.
// The manifest records SHA-256 hashes but not content. However, we can reconstruct
// the pristine version from the npm package cache or git history.
// As a practical approach: save the manifest's version info so the reapply workflow
// knows which GSD version these files came from, enabling npm-based reconstruction.
const meta = {
backed_up_at: new Date().toISOString(),
from_version: manifest.version,
@@ -6863,6 +6944,37 @@ function saveLocalPatches(configDir) {
for (const f of modified) {
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.
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');
}
} 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.');
}
}
}
return modified;
}
@@ -6974,8 +7086,16 @@ function install(isGlobal, runtime = 'claude') {
// Track installation failures
const failures = [];
// Save any locally modified GSD files before they get wiped
saveLocalPatches(targetDir);
// Save any locally modified GSD files before they get wiped.
// The pristine context lets saveLocalPatches populate gsd-pristine/ via
// the install transform pipeline, giving the reapply-patches Step 5
// verifier a real diff baseline (#2998).
saveLocalPatches(targetDir, {
packageSrc: src,
runtime,
pathPrefix,
isGlobal,
});
// Clean up orphaned files from previous versions
cleanupOrphanedFiles(targetDir);
@@ -9071,6 +9191,7 @@ if (process.env.GSD_TEST_MODE) {
validateHookFields,
preserveUserArtifacts,
restoreUserArtifacts,
populatePristineDir,
USER_OWNED_ARTIFACTS,
finishInstall,
trySelfLinkGsdSdk,