From 74a121bb4f0daa82c7d019edf6398f8f4bc03ccf Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 9 Jun 2026 12:51:15 -0400 Subject: [PATCH] fix(#934): reapply verifier handles missing pristine baseline post-rename (#937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#934): reapply verifier handles missing pristine baseline post-rename Gap 1 (verify-reapply-patches.cjs): when backup-meta.json records a pristine_hash for a file but gsd-pristine/ has no corresponding snapshot on disk, the verifier fell to over-broad mode and produced false FAIL_USER_LINES_MISSING. Fix: return advisory OK_NO_BASELINE (non-blocking, exit 0) so the verifier does not block on files it cannot reason about. Gap 2 (new migration 004): migration 003 removed legacy get-shit-done/ runtime files but left gsd-pristine/get-shit-done/ orphan snapshots in place. Those stale snapshots referenced get-shit-done/... key paths that no longer match the active gsd-core/... layout. Fix: add migration 004-prune-stale-pristine-get-shit-done (NOT editing 003, preserving its checksum — ref #670 guard) to remove all files under gsd-pristine/get-shit-done/ as GSD-managed pristine snapshots. Includes tests: bug-934 OK_NO_BASELINE assertions in the verifier test, new installer-migration-prune-stale-pristine.test.cjs, updated installer-migrations baseline-lock checksum for 004. Co-Authored-By: Claude Opus 4.8 * fix(#934): rename migration to satisfy legacy-name guard + mark intentional path refs Rename src/installer-migrations/004-prune-stale-pristine-get-shit-done.cts → 004-prune-stale-pristine-snapshots.cts so the filename no longer contains the forbidden token. Update .gitignore and eslint.config.mjs to track the new built path. Add gsd-allow-legacy-name markers to the remaining intentional uses of the legacy path string in the migration body (lines 3 and 100) and in tests (installer-migration-prune-stale-pristine.test.cjs lines 202 and 226; and the baseline-lock key in installer-migrations.test.cjs:1469). Update the baseline checksum for migration 2026-06-09-prune-stale-pristine-get-shit-done to reflect the two new marker comments added to its body. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 --- .changeset/934-reapply-pristine-baseline.md | 9 + .gitignore | 1 + eslint.config.mjs | 1 + gsd-core/bin/verify-reapply-patches.cjs | 64 +++- gsd-core/workflows/reapply-patches.md | 19 +- .../004-prune-stale-pristine-snapshots.cts | 145 +++++++++ .../bug-2969-verify-reapply-patches.test.cjs | 5 +- ...fy-reapply-patches-pristine-drift.test.cjs | 154 ++++++++++ ...er-migration-prune-stale-pristine.test.cjs | 280 ++++++++++++++++++ tests/installer-migrations.test.cjs | 3 + 10 files changed, 672 insertions(+), 9 deletions(-) create mode 100644 .changeset/934-reapply-pristine-baseline.md create mode 100644 src/installer-migrations/004-prune-stale-pristine-snapshots.cts create mode 100644 tests/installer-migration-prune-stale-pristine.test.cjs diff --git a/.changeset/934-reapply-pristine-baseline.md b/.changeset/934-reapply-pristine-baseline.md new file mode 100644 index 000000000..9b86100a7 --- /dev/null +++ b/.changeset/934-reapply-pristine-baseline.md @@ -0,0 +1,9 @@ +--- +type: Fixed +pr: 935 +--- +Fix `--reapply` verifier false-positives on post-#604-rename installs caused by two gaps in pristine-baseline handling: + +**Gap 1** (`verify-reapply-patches.cjs`): when `backup-meta.json` records a `pristine_hash` for a file but `gsd-pristine/` has no corresponding snapshot on disk, the verifier fell to over-broad mode (every upstream-changed line treated as a user-added requirement) and produced `FAIL_USER_LINES_MISSING` false positives. Fix: return advisory `OK_NO_BASELINE` reason (non-blocking, exit 0) when a recorded hash is present but the pristine file is absent — the verifier cannot reason correctly without a baseline and must not block. + +**Gap 2** (new migration `004-prune-stale-pristine-get-shit-done`): migration 003 removed legacy `get-shit-done/` runtime files but left `gsd-pristine/get-shit-done/` orphan snapshots in place. Those stale snapshots referenced `get-shit-done/...` key paths that no longer match the active `gsd-core/...` layout, contributing to `FAIL_INSTALLED_MISSING` false reports. Fix: add a new migration (not editing 003, to preserve its checksum) that removes all files under `gsd-pristine/get-shit-done/`. (#934) diff --git a/.gitignore b/.gitignore index b431ccedb..337856035 100644 --- a/.gitignore +++ b/.gitignore @@ -113,6 +113,7 @@ build/ /gsd-core/bin/lib/model-profiles.cjs /gsd-core/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs /gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs +/gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs /gsd-core/bin/lib/observability/logger.cjs /gsd-core/bin/lib/active-workstream-store.cjs /gsd-core/bin/lib/adr-parser.cjs diff --git a/eslint.config.mjs b/eslint.config.mjs index 644bf64aa..4662670f1 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -78,6 +78,7 @@ export default tseslint.config( 'gsd-core/bin/lib/federated-config.cjs', 'gsd-core/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs', 'gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs', + 'gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs', 'gsd-core/bin/lib/observability/logger.cjs', 'gsd-core/bin/lib/active-workstream-store.cjs', 'gsd-core/bin/lib/adr-parser.cjs', diff --git a/gsd-core/bin/verify-reapply-patches.cjs b/gsd-core/bin/verify-reapply-patches.cjs index 79e7965bb..a495337c6 100755 --- a/gsd-core/bin/verify-reapply-patches.cjs +++ b/gsd-core/bin/verify-reapply-patches.cjs @@ -165,6 +165,19 @@ const REASON = Object.freeze({ // 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', + // Bug #934: backup-meta.json records a pristine_hash for this file but the + // gsd-pristine/ file is absent from disk. This happens on post-#604-rename + // installs where saveLocalPatches discarded the only pristine candidate + // because its hash did not match the old-release hash (the file changed + // upstream between releases). Without a baseline the verifier cannot + // distinguish user-added lines from upstream-changed lines, so falling to + // over-broad mode would produce FAIL_USER_LINES_MISSING false positives for + // every upstream-removed line. The correct posture is advisory/non-blocking: + // report OK_NO_BASELINE so the caller can log a warning without halting the + // gate on a spurious failure. This is a bounded "cannot reason → do not + // block" rather than "ignore everything" — it only applies when the hash was + // recorded (modern installer) but the file is absent (specific gap). + OK_NO_BASELINE: 'ok_no_baseline', FAIL_INSTALLED_MISSING: 'fail_installed_missing', FAIL_INSTALLED_NOT_REGULAR_FILE: 'fail_installed_not_regular_file', FAIL_READ_ERROR: 'fail_read_error', @@ -208,11 +221,25 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe return result; } + // 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]; + let pristineContent = null; if (pristineDir) { const pristinePath = path.join(pristineDir, relPath); + // Bug #934: track whether the pristine path EXISTS on disk (stat did not + // throw ENOENT). A regular file that fails to read, or a non-file path + // (e.g. a directory accidentally placed at the pristine path), is treated + // as "present but unusable" — we fall to over-broad mode (safe side). + // OK_NO_BASELINE is reserved for the strictly absent case: stat throws, + // meaning the file was never written (the gap the bug describes). + let pristinePathExists = false; try { const stat = fs.statSync(pristinePath); + pristinePathExists = true; // path exists (any type) if (stat.isFile()) { const candidate = fs.readFileSync(pristinePath, 'utf8'); // Bug #3657: if backup-meta.json recorded a pristine_hash for this @@ -226,11 +253,6 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe // 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. @@ -251,8 +273,28 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe pristineContent = candidate; } } + // Non-file at pristinePath (e.g. a directory): stat succeeded so + // pristinePathExists is true; we fall through to over-broad mode below, + // which is safe and conservative. } catch { - // Pristine missing or unreadable — fall through to over-broad mode. + // Pristine stat threw — path is absent (ENOENT) or inaccessible. + // pristinePathExists stays false. + } + + // Bug #934: recordedHash is present (modern installer) but the pristine + // path does not exist on disk at all (stat threw above). This means + // saveLocalPatches recorded a hash but could not write the corresponding + // gsd-pristine/ file (the only candidate was discarded because it was from + // a newer release). Falling to over-broad mode here would treat every + // upstream-changed line as a "user-added line that must survive", producing + // false FAIL_USER_LINES_MISSING for each upstream removal. Since we + // cannot reason correctly without a baseline, the safe answer is advisory/ + // non-blocking: return OK_NO_BASELINE and let the caller decide. + // NOTE: this guard fires ONLY when stat threw (path absent), not when the + // path is present but non-file — in that case over-broad mode is safer. + if (!pristinePathExists && recordedHash) { + result.reason = REASON.OK_NO_BASELINE; + return result; } } @@ -316,9 +358,17 @@ function main() { const drifted = driftedResults.length; const drifted_files = driftedResults.map((r) => r.file); + // Bug #934: aggregate no-baseline files into top-level report fields so the + // workflow can log a warning about files that could not be verified. Like + // drift, this is NOT a failure (exit code stays 0) but gives the caller + // structured data to surface the advisory condition. + const noBaselineResults = results.filter((r) => r.reason === REASON.OK_NO_BASELINE); + const no_baseline = noBaselineResults.length; + const no_baseline_files = noBaselineResults.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', + JSON.stringify({ checked: results.length, failures: failures.length, drifted, drifted_files, no_baseline, no_baseline_files, results }, null, 2) + '\n', ); } else { process.stdout.write(`# Hunk Verification Gate (#2969)\n\n`); diff --git a/gsd-core/workflows/reapply-patches.md b/gsd-core/workflows/reapply-patches.md index b69775eda..94494b33a 100644 --- a/gsd-core/workflows/reapply-patches.md +++ b/gsd-core/workflows/reapply-patches.md @@ -299,11 +299,28 @@ VERIFY_OUTPUT="$(node "${GSD_HOME}/gsd-core/bin/verify-reapply-patches.cjs" "${V VERIFY_STATUS=$? ``` -**Step 5a: drift check** — even when `VERIFY_STATUS` is 0, the report may signal that one or more files were skipped due to pristine-snapshot drift (Bug #3657). Parse the JSON and check: +**Step 5a: drift check** — even when `VERIFY_STATUS` is 0, the report may signal that one or more files were skipped due to pristine-snapshot drift (Bug #3657) or a missing baseline (Bug #934). Parse the JSON and check: ```bash DRIFTED_COUNT="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));process.stdout.write(String(d.drifted||0))")" DRIFTED_FILES="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));(d.drifted_files||[]).forEach(f=>process.stdout.write(f+'\n'))")" +NO_BASELINE_COUNT="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));process.stdout.write(String(d.no_baseline||0))")" +NO_BASELINE_FILES="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));(d.no_baseline_files||[]).forEach(f=>process.stdout.write(f+'\n'))")" +``` + +**If `NO_BASELINE_COUNT` is greater than 0**, emit an advisory warning (non-blocking — the gate still exits 0 for these files). Do NOT halt: + +```text +ADVISORY: {NO_BASELINE_COUNT} file(s) could not be diff-verified because no pristine +baseline exists on disk despite a hash being recorded in backup-meta.json (Bug #934: +the installer discarded the only pristine candidate because it was from a newer release). +These files were skipped rather than false-failed; their user customisations may or +may not have survived the merge. + +Unverified files: + {each path in NO_BASELINE_FILES, one per line, indented two spaces} + +Recommended: manually inspect each file above and confirm your customisations survived. ``` **If `DRIFTED_COUNT` is greater than 0**, STOP and report to the user, then set `DRIFT_DETECTED=true` and halt — do not proceed to 5b or cleanup: diff --git a/src/installer-migrations/004-prune-stale-pristine-snapshots.cts b/src/installer-migrations/004-prune-stale-pristine-snapshots.cts new file mode 100644 index 000000000..c239da07a --- /dev/null +++ b/src/installer-migrations/004-prune-stale-pristine-snapshots.cts @@ -0,0 +1,145 @@ +/** + * Installer migration 004: remove stale gsd-pristine/get-shit-done/ snapshot // gsd-allow-legacy-name + * files after the get-shit-done → gsd-core rename (#604, #934). // gsd-allow-legacy-name + * + * Background: migration 003 removed legacy runtime files from + * get-shit-done/ but did not touch gsd-pristine/get-shit-done/, the // gsd-allow-legacy-name + * parallel directory that holds pristine snapshots captured before the rename. + * These snapshot files are GSD-managed (written by the installer, never by the + * user) and reference stale get-shit-done/... key paths that no longer exist // gsd-allow-legacy-name + * in the active layout. When verify-reapply-patches.cjs looks up a backup entry + * keyed under gsd-core/... it finds no matching gsd-pristine/ snapshot, falls + * to over-broad mode, and reports false FAIL_INSTALLED_MISSING / // gsd-allow-legacy-name + * FAIL_USER_LINES_MISSING for every backed-up pre-rename file (#934). + * + * Fix: walk gsd-pristine/get-shit-done/ and emit remove-managed for each file. // gsd-allow-legacy-name + * These files are always GSD-written snapshots — users never place their own + * files inside gsd-pristine/ — so the classification override + * (managed-pristine) is safe: there is no user content to protect. + * + * Checksum safety: migration 003's body is left untouched. Adding this + * separate migration avoids modifying 003's checksum, which would break + * upgrade state for any user who already applied 003 (root cause of #670). + * + * Per-file approach: the migration framework has no recursive directory-removal + * primitive — all actions operate on individual files. Empty directory shells + * left after removal can be cleaned up manually; this is the intentional ADR-0008 + * limitation. + */ + +import fs from 'node:fs'; +import path from 'node:path'; + +interface MigrationAction { + type: string; + relPath: string; + reason: string; + ownershipEvidence: string; + classification?: string; +} + +interface MigrationPlanContext { + configDir: string; + classifyArtifact(relPath: string): { classification: string; [key: string]: unknown }; +} + +interface InstallerMigration { + id: string; + title: string; + description: string; + introducedIn: string; + scopes: string[]; + destructive: boolean; + plan(ctx: MigrationPlanContext): MigrationAction[]; +} + +function walkPristineFiles(root: string, relDir: string, baseResolved: string, results: string[]): void { + const dir = path.join(root, relDir); + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; // directory absent or unreadable — nothing to do + } + for (const entry of entries) { + // Do not follow symlinks — skip to avoid out-of-tree traversal. + if (entry.isSymbolicLink()) continue; + const relPath = path.posix.join(relDir, entry.name); + // Bounds check: ensure the resolved path stays under configDir. + const resolved = path.resolve(root, relPath); + if (resolved !== baseResolved && !resolved.startsWith(baseResolved + path.sep)) continue; + if (entry.isDirectory()) { + walkPristineFiles(root, relPath, baseResolved, results); + } else if (entry.isFile()) { + results.push(relPath); + } + } +} + +const REASON = 'stale pristine snapshot from legacy get-shit-done/ dir, orphaned by rename migration 003 (#604, #934)'; // gsd-allow-legacy-name + +const migration: InstallerMigration = { + id: '2026-06-09-prune-stale-pristine-get-shit-done', // gsd-allow-legacy-name + title: 'Remove stale gsd-pristine/get-shit-done/ snapshot files (#934)', // gsd-allow-legacy-name + description: + 'Migration 003 removed runtime files from get-shit-done/ but left the matching pristine snapshot ' + // gsd-allow-legacy-name + 'directory gsd-pristine/get-shit-done/ intact. Those snapshots reference stale key paths and cause ' + // gsd-allow-legacy-name + 'verify-reapply-patches false positives (#934). Remove all files under gsd-pristine/get-shit-done/ ' + // gsd-allow-legacy-name + 'as they are GSD-managed snapshots, never user content.', + introducedIn: '1.4.3', + scopes: ['global', 'local'], + destructive: true, + plan(ctx: MigrationPlanContext): MigrationAction[] { + const pristineGsdRoot = path.join(ctx.configDir, 'gsd-pristine', 'get-shit-done'); // gsd-allow-legacy-name + + // Idempotency: if the stale pristine subdir doesn't exist, nothing to do. + if (!fs.existsSync(pristineGsdRoot)) return []; + + // Safety: reject symlinks in ANY ancestor component of the path we will walk + // to prevent following a symlink out of configDir. Check both gsd-pristine/ + // and gsd-pristine/get-shit-done/ — either being a symlink could redirect // gsd-allow-legacy-name + // the walk to an out-of-tree location. + const pristineParent = path.join(ctx.configDir, 'gsd-pristine'); + try { + if (fs.lstatSync(pristineParent).isSymbolicLink()) return []; + } catch { + return []; + } + try { + if (fs.lstatSync(pristineGsdRoot).isSymbolicLink()) return []; // gsd-allow-legacy-name + } catch { + return []; + } + + const baseResolved = path.resolve(ctx.configDir); + const relPaths: string[] = []; + walkPristineFiles(ctx.configDir, path.posix.join('gsd-pristine', 'get-shit-done'), baseResolved, relPaths); // gsd-allow-legacy-name + + const actions: MigrationAction[] = []; + for (const relPath of relPaths) { + // Bounds-check each relPath before emitting any action. + const resolved = path.resolve(ctx.configDir, relPath); + if (resolved !== baseResolved && !resolved.startsWith(baseResolved + path.sep)) continue; + + // These files are GSD-managed pristine snapshots — the installer writes + // them during install/upgrade; users never place personal files inside + // gsd-pristine/. Pass classification: 'managed-pristine' explicitly so + // the framework does not downgrade remove-managed to preserve-user when + // the manifest has no entry (these paths were never in the manifest since + // they live under gsd-pristine/, not the tracked runtime dir). + actions.push({ + type: 'remove-managed', + relPath, + reason: REASON, + ownershipEvidence: + 'GSD-written pristine snapshot under gsd-pristine/get-shit-done/; ' + // gsd-allow-legacy-name + 'installer is the sole author of gsd-pristine/ contents; no user content lives here', + classification: 'managed-pristine', + }); + } + + return actions; + }, +}; + +export = migration; diff --git a/tests/bug-2969-verify-reapply-patches.test.cjs b/tests/bug-2969-verify-reapply-patches.test.cjs index b9746068b..918c147f4 100644 --- a/tests/bug-2969-verify-reapply-patches.test.cjs +++ b/tests/bug-2969-verify-reapply-patches.test.cjs @@ -88,6 +88,7 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => { // 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. + // Bug #934 added OK_NO_BASELINE. assert.deepEqual( Object.keys(REASON).sort(), [ @@ -95,6 +96,7 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => { 'FAIL_INSTALLED_NOT_REGULAR_FILE', 'FAIL_READ_ERROR', 'FAIL_USER_LINES_MISSING', + 'OK_NO_BASELINE', 'OK_NO_SIGNIFICANT_BACKUP_LINES', 'OK_NO_USER_LINES_VS_PRISTINE', 'OK_PRISTINE_DRIFT_DETECTED', @@ -178,7 +180,8 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => { 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']); + // Bug #934: no_baseline + no_baseline_files are additive fields for missing-pristine advisory. + assert.deepEqual(Object.keys(report).sort(), ['checked', 'drifted', 'drifted_files', 'failures', 'no_baseline', 'no_baseline_files', 'results']); const r0 = report.results[0]; assert.deepEqual(Object.keys(r0).sort(), ['file', 'missing', 'reason', 'status']); assert.equal(typeof r0.file, 'string'); diff --git a/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs b/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs index 10293dc6f..463f3d6a8 100644 --- a/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs +++ b/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs @@ -332,6 +332,7 @@ describe('Bug #3657: pristine-drift does not produce false FAIL_USER_LINES_MISSI 'FAIL_INSTALLED_NOT_REGULAR_FILE', 'FAIL_READ_ERROR', 'FAIL_USER_LINES_MISSING', + 'OK_NO_BASELINE', 'OK_NO_SIGNIFICANT_BACKUP_LINES', 'OK_NO_USER_LINES_VS_PRISTINE', 'OK_PRISTINE_DRIFT_DETECTED', @@ -525,3 +526,156 @@ describe('Bug #3657: pristine-drift does not produce false FAIL_USER_LINES_MISSI ); }); }); + +// --------------------------------------------------------------------------- +// Bug #934: OK_NO_BASELINE — pristine dir provided, hash recorded, but file absent +// --------------------------------------------------------------------------- + +describe('Bug #934: OK_NO_BASELINE when recordedHash present but pristine file absent', () => { + + /** + * Core regression: backup-meta.json has a pristine_hash for the file but + * the gsd-pristine/ snapshot is absent from disk (the installer's + * saveLocalPatches discarded the only candidate because its hash did not + * match the old-release hash — the file changed upstream between releases). + * Without the fix the verifier falls to over-broad mode and treats every + * upstream-removed line as a "user-added line that must survive", producing + * FAIL_USER_LINES_MISSING false positives. + * With the fix the verifier returns OK_NO_BASELINE (non-blocking, advisory). + */ + test('exits 0 with reason=OK_NO_BASELINE when recordedHash present but pristine absent', () => { + resetFixture(); + + const FILE = 'gsd-core/workflows/execute-phase.md'; + + // The backup contains both the old upstream content and the user's line. + const backupContent = + 'upstream line that was present in 1.4.0 but removed in 1.4.2 release\n' + + 'another upstream line removed upstream between gsd-core releases here\n' + + 'model: sonnet in frontmatter — this is the real user customisation line\n'; + + // The installed file has the new upstream content + the user's real line. + const installedContent = + 'brand-new upstream line that replaced the old content in gsd-core 1.4.2\n' + + 'model: sonnet in frontmatter — this is the real user customisation line\n'; + + // backup-meta.json records a hash (modern installer) but gsd-pristine/ is absent. + writeBackupMeta({ pristine_hashes: { [FILE]: 'sha256:deadbeef00000000000000000000000000000000000000000000000000000001' } }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), installedContent); + // Deliberately do NOT write a pristine file — this is the gap-1 scenario. + + const { status, report } = runVerifier(); + + // Must exit 0: cannot reason without baseline → non-blocking advisory. + assert.equal(status, 0, `expected exit 0; got ${status}; report=${JSON.stringify(report)}`); + assert.equal(report.failures, 0, `expected 0 failures; got ${report.failures}`); + const r0 = report.results[0]; + assert.equal(r0.status, 'ok', `expected status ok; got ${r0.status}`); + assert.equal(r0.reason, REASON.OK_NO_BASELINE, + `expected OK_NO_BASELINE; got ${r0.reason}`); + assert.deepEqual(r0.missing, []); + }); + + /** + * Counter-test: when pristine is absent but NO recordedHash is present + * (pre-fix installer that never wrote backup-meta.json), the verifier must + * still fall to over-broad mode — the old behaviour for untracked backups. + * OK_NO_BASELINE must NOT fire in this case. + */ + test('falls through to over-broad mode when pristine absent AND no recordedHash', () => { + resetFixture(); + + const FILE = 'gsd-core/workflows/plan-phase.md'; + const droppedLine = 'user-added instruction that was dropped from the install output'; + const backupContent = + 'stock upstream line long enough to be significant in the file\n' + + droppedLine + '\n'; + const installedContent = 'stock upstream line long enough to be significant in the file\n'; + + // No backup-meta.json — simulates pre-fix installer with no hash records. + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), installedContent); + // No pristine file. + + const { status, report } = runVerifier(); + + // Over-broad mode catches the genuinely dropped user line. + assert.equal(status, 1, 'over-broad mode should catch the dropped user line'); + assert.equal(report.failures, 1); + const r0 = report.results[0]; + assert.equal(r0.status, 'fail'); + assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING); + assert.ok(r0.missing.includes(droppedLine), + `dropped line must appear in .missing[]; got ${JSON.stringify(r0.missing)}`); + // Must NOT be OK_NO_BASELINE — that only fires when a hash WAS recorded. + assert.notEqual(r0.reason, REASON.OK_NO_BASELINE); + }); + + /** + * Presence check: when pristine IS present AND hash matches, the normal + * flow must proceed (not short-circuit to OK_NO_BASELINE). + * A real dropped user line must still be caught. + */ + test('does not short-circuit to OK_NO_BASELINE when pristine exists and hash matches', () => { + resetFixture(); + + const FILE = 'gsd-core/workflows/plan-phase.md'; + const pristineContent = 'stock upstream line long enough to be significant content\n'; + const droppedLine = 'user customisation that was genuinely dropped from the merged output'; + const backupContent = pristineContent + droppedLine + '\n'; + const installedContent = pristineContent; // user line dropped — real failure + + writeBackupMeta({ pristine_hashes: { [FILE]: sha256(pristineContent) } }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), installedContent); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runVerifier(); + + assert.equal(status, 1, 'real dropped user line must be caught'); + assert.equal(report.failures, 1); + const r0 = report.results[0]; + assert.equal(r0.status, 'fail'); + assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING); + assert.notEqual(r0.reason, REASON.OK_NO_BASELINE); + assert.ok(r0.missing.includes(droppedLine)); + }); + + /** + * When --pristine-dir is NOT provided at all (old CLI invocation without the + * flag), the OK_NO_BASELINE path must never fire — there is no pristine dir + * context to consult and the old over-broad behaviour must be preserved. + */ + test('does not return OK_NO_BASELINE when --pristine-dir is not provided', () => { + resetFixture(); + + const FILE = 'gsd-core/workflows/execute-phase.md'; + const backupContent = + 'upstream line removed in newer version but present in backup\n' + + 'model: sonnet — user customisation line in the backup file\n'; + const installedContent = + 'replacement upstream line in the newer release version\n' + + 'model: sonnet — user customisation line in the backup file\n'; + + // Record a hash — but no pristine dir will be passed to the verifier. + writeBackupMeta({ pristine_hashes: { [FILE]: 'sha256:deadbeef00000000000000000000000000000000000000000000000000000001' } }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), installedContent); + + // Run without --pristine-dir flag. + const { status, report } = runVerifier({ pristine: false }); + + // Over-broad mode: every significant backup line is required. + // "upstream line removed in newer version but present in backup" is NOT in + // the installed content → over-broad mode FAILS this file (exit 1). + // OK_NO_BASELINE must NOT fire — there was no pristine dir to consult. + assert.equal(status, 1, `over-broad mode should fail (upstream-removed line absent); got ${status}`); + const r0 = report.results[0]; + assert.equal(r0.status, 'fail', `expected fail status; got ${r0.status}`); + assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING, + `expected FAIL_USER_LINES_MISSING from over-broad mode; got ${r0.reason}`); + assert.notEqual(r0.reason, REASON.OK_NO_BASELINE, + `OK_NO_BASELINE must not fire when --pristine-dir is not provided`); + }); +}); diff --git a/tests/installer-migration-prune-stale-pristine.test.cjs b/tests/installer-migration-prune-stale-pristine.test.cjs new file mode 100644 index 000000000..173db4349 --- /dev/null +++ b/tests/installer-migration-prune-stale-pristine.test.cjs @@ -0,0 +1,280 @@ +'use strict'; + +/** + * TDD tests for installer migration 004: + * 2026-06-09-prune-stale-pristine-get-shit-done // gsd-allow-legacy-name + * + * Verifies plan() logic for: + * 1. Stale pristine subdir absent -> empty plan (idempotency) + * 2. Stale pristine subdir present -> remove-managed actions emitted for each file + * 3. Stale pristine root is a symlink -> empty plan (symlink safety) + * 4. Symlinked entry inside stale pristine dir is NOT emitted + * 5. Mixed files: all get remove-managed (no user-file classification needed) + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +// Load compiled module (build:lib compiles src/*.cts -> gsd-core/bin/lib/*.cjs) +const migration = require('../gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs'); + +function createTempDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-migration-004-test-')); +} + +function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup in migration test; no helpers import available + fs.rmSync(dir, { recursive: true, force: true }); +} + +function writeFile(root, relPath, content) { + const fullPath = path.join(root, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, content, 'utf8'); +} + +function writeManifest(root, files) { + fs.writeFileSync( + path.join(root, 'gsd-file-manifest.json'), + JSON.stringify({ + version: '1.3.0', + timestamp: '2026-06-01T00:00:00.000Z', + mode: 'full', + files, + }, null, 2), + 'utf8' + ); +} + +// Build a plan context using the real installer-migrations classifyArtifact. +const { + classifyArtifact: realClassifyArtifact, + readInstallManifest, +} = require('../gsd-core/bin/lib/installer-migrations.cjs'); + +function makePlanCtx(configDir) { + const manifest = readInstallManifest(configDir); + return { + configDir, + classifyArtifact: (relPath) => realClassifyArtifact(configDir, relPath, manifest), + }; +} + +// --------------------------------------------------------------------------- +// Metadata +// --------------------------------------------------------------------------- + +describe('migration 004 metadata', () => { + test('exports a single migration object with required fields', () => { + assert.equal(typeof migration, 'object'); + assert.equal(typeof migration.id, 'string'); + assert.ok(migration.id.length > 0, 'id must be non-empty'); + assert.equal(typeof migration.title, 'string'); + assert.equal(typeof migration.description, 'string'); + assert.equal(typeof migration.introducedIn, 'string'); + assert.ok(Array.isArray(migration.scopes), 'scopes must be an array'); + assert.ok(migration.scopes.includes('global'), 'scopes must include global'); + assert.ok(migration.scopes.includes('local'), 'scopes must include local'); + assert.strictEqual(migration.destructive, true); + assert.equal(typeof migration.plan, 'function'); + }); + + test('id contains expected date prefix', () => { + assert.ok(migration.id.startsWith('2026-06-09-'), `id should start with date prefix, got: ${migration.id}`); + }); + + test('id references prune-stale-pristine', () => { + assert.ok( + migration.id.includes('prune-stale-pristine') || migration.id.includes('pristine'), + `id should reference pristine pruning, got: ${migration.id}`, + ); + }); +}); + +// --------------------------------------------------------------------------- +// Case 1: stale pristine subdir absent -> empty plan +// --------------------------------------------------------------------------- + +describe('plan() — stale pristine subdir absent', () => { + test('returns empty array when gsd-pristine/get-shit-done/ does not exist', () => { // gsd-allow-legacy-name + const configDir = createTempDir(); + try { + // Only gsd-pristine/gsd-core/ exists — no legacy subdir. + writeFile(configDir, 'gsd-pristine/gsd-core/workflows/plan.md', 'pristine snapshot\n'); + writeManifest(configDir, {}); + + const actions = migration.plan(makePlanCtx(configDir)); + assert.deepEqual(actions, []); + } finally { + cleanup(configDir); + } + }); + + test('returns empty array when gsd-pristine/ does not exist at all', () => { + const configDir = createTempDir(); + try { + writeManifest(configDir, {}); + const actions = migration.plan(makePlanCtx(configDir)); + assert.deepEqual(actions, []); + } finally { + cleanup(configDir); + } + }); +}); + +// --------------------------------------------------------------------------- +// Case 2: stale pristine subdir present -> remove-managed for each file +// --------------------------------------------------------------------------- + +describe('plan() — stale pristine files present', () => { + test('emits remove-managed for each file under gsd-pristine/get-shit-done/', () => { // gsd-allow-legacy-name + const configDir = createTempDir(); + try { + writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', 'old pristine\n'); // gsd-allow-legacy-name + writeFile(configDir, 'gsd-pristine/get-shit-done/skills/gsd-foo/SKILL.md', 'old skill\n'); // gsd-allow-legacy-name + writeManifest(configDir, {}); + + const actions = migration.plan(makePlanCtx(configDir)); + assert.equal(actions.length, 2, `expected 2 actions, got ${actions.length}`); + for (const action of actions) { + assert.equal(action.type, 'remove-managed', `expected remove-managed, got ${action.type}`); + assert.ok( + action.relPath.replace(/\\/g, '/').startsWith('gsd-pristine/get-shit-done/'), // gsd-allow-legacy-name + `relPath should start with gsd-pristine/get-shit-done/, got: ${action.relPath}`, // gsd-allow-legacy-name + ); + assert.equal(typeof action.reason, 'string'); + assert.ok(action.reason.length > 0, 'reason must not be empty'); + assert.equal(typeof action.ownershipEvidence, 'string'); + assert.ok(action.ownershipEvidence.length > 0, 'ownershipEvidence must not be empty'); + } + } finally { + cleanup(configDir); + } + }); + + test('emits exactly one remove-managed per file (correct relPaths)', () => { + const configDir = createTempDir(); + try { + writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/execute-phase.md', 'pristine\n'); // gsd-allow-legacy-name + writeManifest(configDir, {}); + + const actions = migration.plan(makePlanCtx(configDir)); + assert.equal(actions.length, 1); + const relPathNorm = actions[0].relPath.replace(/\\/g, '/'); + assert.equal(relPathNorm, 'gsd-pristine/get-shit-done/workflows/execute-phase.md'); // gsd-allow-legacy-name + } finally { + cleanup(configDir); + } + }); + + test('actions include classification override to managed-pristine', () => { + const configDir = createTempDir(); + try { + writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', 'pristine snapshot\n'); // gsd-allow-legacy-name + writeManifest(configDir, {}); + + const actions = migration.plan(makePlanCtx(configDir)); + assert.equal(actions.length, 1); + // The action must carry classification:'managed-pristine' so the framework + // does not downgrade remove-managed to preserve-user (the file is not in + // the manifest so classify() would return 'unknown'). + assert.equal(actions[0].classification, 'managed-pristine', + 'action must carry classification:managed-pristine override'); + } finally { + cleanup(configDir); + } + }); +}); + +// --------------------------------------------------------------------------- +// Case 3: stale pristine root is a symlink -> plan returns [] (symlink safety) +// --------------------------------------------------------------------------- + +describe('plan() — stale pristine root is a symlink', () => { + test('returns empty array when gsd-pristine/get-shit-done/ is a symlink', () => { // gsd-allow-legacy-name + const configDir = createTempDir(); + const externalDir = createTempDir(); + try { + writeFile(externalDir, 'workflows/plan.md', 'pristine content\n'); + // Create gsd-pristine/ as a real dir but make get-shit-done/ a symlink. // gsd-allow-legacy-name + fs.mkdirSync(path.join(configDir, 'gsd-pristine'), { recursive: true }); + const legacyLink = path.join(configDir, 'gsd-pristine', 'get-shit-done'); // gsd-allow-legacy-name + fs.symlinkSync(externalDir, legacyLink); + writeManifest(configDir, {}); + + const actions = migration.plan(makePlanCtx(configDir)); + assert.deepEqual(actions, [], 'plan() must return [] when stale pristine root is a symlink'); + } finally { + cleanup(configDir); + cleanup(externalDir); + } + }); +}); + +// --------------------------------------------------------------------------- +// Case 4: symlinked entry inside stale pristine dir is skipped +// --------------------------------------------------------------------------- + +describe('plan() — symlinked entry inside stale pristine dir is skipped', () => { + test('symlinked file inside stale pristine dir is not included in plan actions', () => { + const configDir = createTempDir(); + const externalTarget = createTempDir(); + try { + // A real file inside gsd-pristine/get-shit-done/ // gsd-allow-legacy-name + writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', 'real pristine\n'); // gsd-allow-legacy-name + + // A symlink inside the same dir pointing to external target. + const externalFile = path.join(externalTarget, 'external.md'); + fs.writeFileSync(externalFile, 'external content\n', 'utf8'); + const symlinkPath = path.join(configDir, 'gsd-pristine', 'get-shit-done', 'workflows', 'symlinked.md'); // gsd-allow-legacy-name + fs.symlinkSync(externalFile, symlinkPath); + + writeManifest(configDir, {}); + + const actions = migration.plan(makePlanCtx(configDir)); + + // Only the real file should appear; the symlinked entry must be skipped. + assert.equal(actions.length, 1, `expected 1 action (real file only), got ${actions.length}`); + const hasSymlinked = actions.some((a) => a.relPath.includes('symlinked')); + assert.equal(hasSymlinked, false, 'symlinked entry must not appear in plan actions'); + } finally { + cleanup(configDir); + cleanup(externalTarget); + } + }); +}); + +// --------------------------------------------------------------------------- +// Integration: plan goes through planInstallerMigrations + applyInstallerMigrationPlan +// Files are actually removed from disk. +// --------------------------------------------------------------------------- + +describe('plan() — integration: stale pristine files are removed', () => { + test('stale gsd-pristine/get-shit-done/ file is removed after apply', () => { // gsd-allow-legacy-name + const configDir = createTempDir(); + try { + const fileContent = 'old pristine snapshot content written by gsd installer\n'; + writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', fileContent); // gsd-allow-legacy-name + writeManifest(configDir, {}); + + const { planInstallerMigrations, applyInstallerMigrationPlan } = require('../gsd-core/bin/lib/installer-migrations.cjs'); + const plan = planInstallerMigrations({ + configDir, + migrations: [migration], + scope: 'global', + }); + + assert.equal(plan.blocked.length, 0, `expected no blocked actions; got ${JSON.stringify(plan.blocked)}`); + applyInstallerMigrationPlan({ configDir, plan }); + + // File must be gone after apply. + const stillThere = fs.existsSync(path.join(configDir, 'gsd-pristine', 'get-shit-done', 'workflows', 'plan.md')); // gsd-allow-legacy-name + assert.equal(stillThere, false, 'stale pristine file must be removed after apply'); + } finally { + cleanup(configDir); + } + }); +}); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index 4367f8f15..10c51f6f8 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -1465,6 +1465,9 @@ test('shipped installer-migration checksums are locked to a committed baseline ( 'sha256:5ce55294aa02f25758f604a569c899a6d2d060299189f5f447f68d8033157058', '2026-06-02-rename-get-shit-done-to-gsd-core': 'sha256:3a9f1d97f64097fb313203d19c6d93a187a38df61dd299afa5eef73e16124e95', + // Migration 004: prune stale gsd-pristine/get-shit-done/ snapshots (#934) // gsd-allow-legacy-name + '2026-06-09-prune-stale-pristine-get-shit-done': // gsd-allow-legacy-name + 'sha256:6555dd044659276fbc204e81793cd92c5315d54e7316bcdd82d2c98d15a7e9e8', }; const { DEFAULT_MIGRATIONS_DIR, migrationChecksum: computeChecksum } = require('../gsd-core/bin/lib/installer-migrations.cjs');