diff --git a/.changeset/agile-wasps-chatter.md b/.changeset/agile-wasps-chatter.md new file mode 100644 index 000000000..f8e383d9f --- /dev/null +++ b/.changeset/agile-wasps-chatter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4373 +--- +**`/gsd-update --reapply` no longer re-grafts customizations that upstream already adopted** — the documented `Incorporated` per-file status is now computed by a deterministic pre-flight classifier (hash-validated pristine baseline + every significant user-added line already present verbatim in the new version), so superseded patches are reported as already upstream instead of being silently re-applied on every future update cycle. (#4136) diff --git a/gsd-core/bin/verify-reapply-patches.cjs b/gsd-core/bin/verify-reapply-patches.cjs index c052fc3ec..fedc65039 100755 --- a/gsd-core/bin/verify-reapply-patches.cjs +++ b/gsd-core/bin/verify-reapply-patches.cjs @@ -17,8 +17,11 @@ * # false-positive halts beat silent successes * # on lost content) * [--json] # emit JSON report instead of human text + * [--classify] # pre-merge mode: classify each backed-up + * # file as incorporated / needs_merge / + * # unknown (#4136); always exits 0 * - * Exit codes: + * Exit codes (default gate mode): * 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) @@ -27,6 +30,15 @@ * 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. + * + * Bug #4136 adds --classify (pre-merge mode, always exit 0 — informational; + * the binding gate remains the post-merge default run): per backed-up file, + * decides whether the user's modification was already adopted upstream + * ("Incorporated", reapply-patches.md Step 4 item 6). The classification is + * ONLY produced from a hash-validated pristine baseline with every + * significant user-added line present verbatim in the freshly installed + * version — a false Incorporated silently retires a live customization, + * which is worse than no Incorporated at all. */ const fs = require('node:fs'); @@ -43,16 +55,17 @@ const SIGNIFICANT_MIN_CHARS = 12; const GSD_HOOK_VERSION_LINE_RE = /^(?:\/\/|#)\s*gsd-hook-version:\s*\S+\s*$/i; function parseArgs(argv) { - const opts = { patchesDir: null, configDir: null, pristineDir: null, json: false }; + const opts = { patchesDir: null, configDir: null, pristineDir: null, json: false, classify: 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 === '--classify') opts.classify = true; else if (arg === '--help' || arg === '-h') { process.stdout.write( - 'usage: verify-reapply-patches.cjs --patches-dir --config-dir [--pristine-dir ] [--json]\n', + 'usage: verify-reapply-patches.cjs --patches-dir --config-dir [--pristine-dir ] [--json] [--classify]\n', ); throw new ExitError(0); } else { @@ -183,12 +196,134 @@ const REASON = Object.freeze({ // 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', + // Bug #4136: an on-disk gsd-pristine/ snapshot exists for this file but + // backup-meta.json records no pristine_hashes entry for it (older + // installer), so nothing confirms the snapshot is the baseline the backup + // was captured against — it could be a drifted newer-version snapshot the + // #3657 guard cannot see. The default gate still uses it as the diff + // baseline (pre-#3657 behaviour, unchanged); --classify refuses to confirm + // adoption on an unvalidated baseline, so a file is never classified + // Incorporated on the snapshot's say-so alone. + OK_UNVALIDATED_BASELINE: 'ok_unvalidated_baseline', 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', }); +/** + * Bug #4136: stable per-file classification codes for --classify (pre-merge) + * mode. Tests assert via `assert.equal(result.classification, CLASSIFICATION.X)` + * rather than regex-matching prose, mirroring the REASON enum contract. + * + * INCORPORATED — hash-validated pristine baseline confirms that every + * significant user-added line is already present verbatim + * in the freshly installed version: upstream adopted the + * customization. The workflow must NOT re-apply the diff. + * NEEDS_MERGE — validated baseline, but at least one significant + * user-added line is absent from the fresh install; the + * missing lines are listed for the merge step. + * UNKNOWN — no confirmable baseline (drift #3657, absent #934, + * unvalidated, or no --pristine-dir), zero significant + * user-added delta, or a structural failure. Never + * Incorporated. + */ +const CLASSIFICATION = Object.freeze({ + INCORPORATED: 'incorporated', + NEEDS_MERGE: 'needs_merge', + UNKNOWN: 'unknown', +}); + +/** + * Typed outcome of resolving one file's pristine baseline. Shared by the + * post-merge gate (verifyFile) and the pre-merge classifier (classifyFile) + * so the two can never drift on what counts as a usable baseline (#4136). + */ +const PRISTINE_RESOLUTION = Object.freeze({ + VALIDATED: 'validated', // recorded hash matches the resolved snapshot + UNVALIDATED: 'unvalidated', // snapshot present, no recorded hash to confirm it + DRIFTED: 'drifted', // recorded hash mismatches the snapshot (#3657) + ABSENT_RECORDED: 'absent_recorded', // recorded hash, no snapshot anywhere (#934/#4145) + OVERBROAD: 'overbroad', // no pristine dir, or a non-file at the path +}); + +/** + * Resolve the pristine baseline for one backed-up file, applying the #3657 + * drift guard, the #4145 hash-first recovery, and the #934 absent-baseline + * guard. Extracted from verifyFile's inline block so --classify reasons over + * the exact same baseline semantics the post-merge gate enforces. + */ +function resolvePristineBaseline({ relPath, pristineDir, pristineHashes }) { + const hashKey = relPath.replace(/\\/g, '/'); + const recordedHash = pristineHashes && pristineHashes[hashKey]; + if (pristineDir) { + const pristinePath = path.join(pristineDir, relPath); + let pristinePathExists = false; + try { + const stat = fs.statSync(pristinePath); + pristinePathExists = true; // path exists (any type) + if (stat.isFile()) { + const candidate = fs.readFileSync(pristinePath, 'utf8'); + if (recordedHash) { + if (sha256(candidate) === recordedHash) { + // Hash matches: the on-disk pristine is the correct baseline. + return { resolution: PRISTINE_RESOLUTION.VALIDATED, content: candidate }; + } + // Hash mismatch: the on-disk gsd-pristine/ was refreshed to a newer + // GSD version after the backup was captured (Bug #3657). Using it as + // the diff baseline would invert the delta and produce false + // FAIL_USER_LINES_MISSING reports. + return { resolution: PRISTINE_RESOLUTION.DRIFTED, content: null }; + } + // No recorded hash for this file (older installer or absent + // backup-meta) — the default gate uses the on-disk pristine as-is + // (pre-fix behaviour); --classify treats it as unvalidated. + return { resolution: PRISTINE_RESOLUTION.UNVALIDATED, content: candidate }; + } + // Non-file at pristinePath (e.g. a directory): fall through to + // over-broad mode below, which is safe and conservative. + } catch { + // Pristine stat threw — path is absent (ENOENT) or inaccessible. + // pristinePathExists stays false. + } + + // Bug #4145: the canonical join missed, but the recorded hash is the + // baseline authority the #3657 drift guard already trusts. Before + // reporting ABSENT_RECORDED, scan gsd-pristine/ for byte-identical content + // (an earlier release may have stored the snapshot without the gsd-core/ + // prefix). An exact sha-256 match cannot be the wrong baseline, and + // gsd-pristine/ holds only backed-up files, so the scan is small. The + // canonical path itself is excluded — a mismatching file at the joined + // path is drift (#3657), never re-adopted through the scan. A recovered + // baseline is hash-confirmed by construction, so it VALIDATES. + if (!pristinePathExists && recordedHash) { + try { + const recoveredRel = findPristineByHash(pristineDir, recordedHash, hashKey); + if (recoveredRel) { + return { + resolution: PRISTINE_RESOLUTION.VALIDATED, + content: fs.readFileSync(path.join(pristineDir, recoveredRel), 'utf8'), + }; + } + } catch { + // scan or read failure — fall through to the ABSENT_RECORDED posture + } + } + + if (pristinePathExists) { + // Present but not a regular file — over-broad mode is the safe side. + return { resolution: PRISTINE_RESOLUTION.OVERBROAD, content: null }; + } + // Bug #934: recordedHash is present (modern installer) but no + // hash-matching pristine exists anywhere under gsd-pristine/ (the stat + // missed and the #4145 recovery found nothing). + if (recordedHash) { + return { resolution: PRISTINE_RESOLUTION.ABSENT_RECORDED, content: null }; + } + } + return { resolution: PRISTINE_RESOLUTION.OVERBROAD, content: null }; +} + /** * #4086: resolve where a backed-up file's INSTALLED counterpart lives. * Primary is the config-dir-relative join (the manifest key's native form). @@ -292,102 +427,32 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe // 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]; + // #4136: the inline baseline block (stat/read + #3657 drift guard + #4145 + // hash-first recovery + #934 absent guard) moved into resolvePristineBaseline + // so the pre-merge classifier reasons over the exact same semantics. + const { resolution, content: pristineContent } = + resolvePristineBaseline({ relPath, pristineDir, pristineHashes }); - 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 - // 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. - 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; - } - } - // 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 stat threw — path is absent (ENOENT) or inaccessible. - // pristinePathExists stays false. - } - - // Bug #4145: the canonical join missed, but the recorded hash is the - // baseline authority the #3657 drift guard already trusts. Before - // reporting OK_NO_BASELINE, scan gsd-pristine/ for byte-identical content - // (an earlier release may have stored the snapshot without the gsd-core/ - // prefix). An exact sha-256 match cannot be the wrong baseline, and - // gsd-pristine/ holds only backed-up files, so the scan is small. The - // canonical path itself is excluded — a mismatching file at the joined - // path is drift (#3657), never re-adopted through the scan. - if (!pristinePathExists && recordedHash) { - try { - const recoveredRel = findPristineByHash(pristineDir, recordedHash, hashKey); - if (recoveredRel) { - pristineContent = fs.readFileSync(path.join(pristineDir, recoveredRel), 'utf8'); - pristinePathExists = true; - } - } catch { - // scan or read failure — fall through to the OK_NO_BASELINE posture - } - } - - // Bug #934: recordedHash is present (modern installer) but no hash-matching - // pristine exists anywhere under gsd-pristine/ (stat threw above AND the - // #4145 scan found nothing). 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 the baseline path is absent (stat threw - // and nothing matched by hash), 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; - } + // Bug #3657: the resolved snapshot hash-mismatches the recorded baseline. + // Skip the file with a diagnostic code rather than diffing against the + // wrong baseline (a re-anchor or git-aware baseline step is required). + if (resolution === PRISTINE_RESOLUTION.DRIFTED) { + result.reason = REASON.OK_PRISTINE_DRIFT_DETECTED; + return result; } + // Bug #934 / #4145: a hash was recorded (modern installer) but no + // hash-matching pristine exists anywhere under gsd-pristine/ — + // advisory/non-blocking, the caller logs a warning. + if (resolution === PRISTINE_RESOLUTION.ABSENT_RECORDED) { + result.reason = REASON.OK_NO_BASELINE; + return result; + } + + // VALIDATED / UNVALIDATED keep the gate's pre-#4136 semantics: the resolved + // content (null for OVERBROAD) feeds computeUserAddedLines, whose no-pristine + // branch is the over-broad fallback. + const userAdded = computeUserAddedLines(backupContent, pristineContent); if (userAdded.length === 0) { // Backup and pristine match exactly (or no significant content) — nothing @@ -410,6 +475,101 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe return result; } +/** + * Bug #4136: pre-merge classification for reapply-patches Step 4. Runs over + * the SAME fixture state the gate sees (patches dir + freshly installed + * config dir + optional pristine dir) but BEFORE any merge, answering the + * question the gate cannot: is this file's user modification already adopted + * upstream, such that the workflow should leave the installed file untouched + * (status `Incorporated`) instead of re-grafting the diff? + * + * The classification is deliberately conservative in the direction the issue + * demands ("a false Incorporated is worse than no Incorporated"): + * - only a hash-VALIDATED pristine baseline can confirm adoption — drift + * (#3657), absent (#934/#4145), unvalidated (no recorded hash), and a + * missing --pristine-dir all classify UNKNOWN; + * - every significant user-added line (structural/trivial lines excluded by + * isSignificantLine) must be present verbatim in the fresh install — a + * signature-looking short line or a code fence matching anywhere in the + * new file proves nothing; + * - a backup with zero significant delta vs the validated pristine is + * UNKNOWN (the workflow's Critical invariant: a backed-up file is never + * concluded to have "no custom content", and Incorporated is a positive + * adoption finding, not a skip); + * - structural failures are reported with their FAIL_* reason but never + * gate the run — the binding enforcement point stays the post-merge gate. + */ +function classifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashes, skillsRedirect }) { + const backupPath = path.join(patchesDir, relPath); + const installedPath = resolveInstalledPath(configDir, relPath, skillsRedirect); + const result = { file: relPath, classification: CLASSIFICATION.UNKNOWN, reason: null, missing: [] }; + + if (!fs.existsSync(backupPath) || !fs.statSync(backupPath).isFile()) { + return result; // walked entry no longer exists — non-fatal + } + + let installedStat; + try { + installedStat = fs.statSync(installedPath); + } catch { + result.reason = REASON.FAIL_INSTALLED_MISSING; + return result; + } + if (!installedStat.isFile()) { + 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.reason = REASON.FAIL_READ_ERROR; + return result; + } + + const { resolution, content: pristineContent } = + resolvePristineBaseline({ relPath, pristineDir, pristineHashes }); + + if (resolution === PRISTINE_RESOLUTION.DRIFTED) { + result.reason = REASON.OK_PRISTINE_DRIFT_DETECTED; + return result; + } + if (resolution === PRISTINE_RESOLUTION.ABSENT_RECORDED) { + result.reason = REASON.OK_NO_BASELINE; + return result; + } + if (resolution === PRISTINE_RESOLUTION.UNVALIDATED) { + result.reason = REASON.OK_UNVALIDATED_BASELINE; + return result; + } + if (resolution !== PRISTINE_RESOLUTION.VALIDATED) { + // OVERBROAD: no --pristine-dir (two-way fallback) or a non-file at the + // pristine path. Without a baseline there is no adoption evidence. + return result; + } + + const userAdded = computeUserAddedLines(backupContent, pristineContent); + if (userAdded.length === 0) { + result.reason = REASON.OK_NO_USER_LINES_VS_PRISTINE; + return result; + } + + for (const line of userAdded) { + if (!installedContent.includes(line)) { + result.missing.push(line.trim()); + } + } + if (result.missing.length === 0) { + result.classification = CLASSIFICATION.INCORPORATED; + } else { + result.classification = CLASSIFICATION.NEEDS_MERGE; + } + return result; +} + function main() { const opts = parseArgs(process.argv.slice(2)); if (!opts.patchesDir || !opts.configDir) { @@ -429,6 +589,55 @@ function main() { // #4086: skills-root fallback for runtimes whose skills kind declares a // `home` override outside configDir (codex global → $HOME/.agents/skills). const skillsRedirect = resolveSkillsRedirect(opts.configDir); + + // Bug #4136: --classify is the pre-merge mode the workflow runs in Step 4 + // to decide which files NOT to merge. It is informational — always exit 0 — + // because the binding enforcement stays with the post-merge gate below; + // a needs_merge or unknown outcome is direction for the merge step, not a + // failure, and halting here would block the very merge that resolves it. + if (opts.classify) { + const classifyResults = files.map((relPath) => + classifyFile({ + relPath, + patchesDir: opts.patchesDir, + configDir: opts.configDir, + pristineDir: opts.pristineDir, + pristineHashes, + skillsRedirect, + }), + ); + const incorporatedResults = classifyResults.filter( + (r) => r.classification === CLASSIFICATION.INCORPORATED, + ); + const incorporated = incorporatedResults.length; + const incorporated_files = incorporatedResults.map((r) => r.file); + + if (opts.json) { + process.stdout.write( + JSON.stringify({ checked: classifyResults.length, incorporated, incorporated_files, results: classifyResults }, null, 2) + '\n', + ); + } else { + process.stdout.write(`# Reapply Patch Classification (#4136)\n\n`); + process.stdout.write(`Checked: ${classifyResults.length} file(s)\n`); + process.stdout.write(`Incorporated: ${incorporated} file(s)\n`); + for (const f of incorporated_files) { + process.stdout.write(` incorporated: ${f}\n`); + } + for (const r of classifyResults) { + if (r.classification === CLASSIFICATION.NEEDS_MERGE) { + process.stdout.write(` needs merge: ${r.file}\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`); + } + } + } + } + return 0; + } + const results = files.map((relPath) => verifyFile({ relPath, @@ -490,4 +699,4 @@ if (require.main === module) { runMain(main); } -module.exports = { computeUserAddedLines, isSignificantLine, verifyFile, walk, REASON, readPristineHashes, sha256, resolveInstalledPath, resolveSkillsRedirect }; +module.exports = { computeUserAddedLines, isSignificantLine, verifyFile, classifyFile, walk, REASON, CLASSIFICATION, PRISTINE_RESOLUTION, resolvePristineBaseline, readPristineHashes, sha256, resolveInstalledPath, resolveSkillsRedirect }; diff --git a/gsd-core/workflows/reapply-patches.md b/gsd-core/workflows/reapply-patches.md index 0b95f333f..a36a94e79 100644 --- a/gsd-core/workflows/reapply-patches.md +++ b/gsd-core/workflows/reapply-patches.md @@ -192,6 +192,58 @@ If neither git history nor pristine snapshots are available, fall back to two-wa ## Step 4: Merge each file +### Step 4 pre-flight: classify superseded customizations (#4136) + +Before merging anything, run the deterministic classifier. It computes, per backed-up file, +whether the user's added lines (diff of the backup against the hash-validated pristine +baseline) are ALREADY present verbatim in the newly installed version — i.e. upstream +adopted the customization. That is the only ground on which the `Incorporated` status +below is valid. Never conclude `Incorporated` from a signature line, a heading, or general +resemblance: the classifier excludes structural/trivial lines and requires every +significant user-added line to be present, because a false `Incorporated` silently retires +a live customization. + +```bash +PRISTINE_DIR="${CONFIG_DIR}/gsd-pristine" + +# Build args as a bash array so paths with spaces survive expansion intact. +CLASSIFY_ARGS=( + --patches-dir "$PATCHES_DIR" + --config-dir "$CONFIG_DIR" +) +if [ -d "$PRISTINE_DIR" ]; then + CLASSIFY_ARGS+=(--pristine-dir "$PRISTINE_DIR") +fi +CLASSIFY_ARGS+=(--classify --json) + +# Informational: exits 0. The binding gate is Step 5a's post-merge verification run. +CLASSIFY_OUTPUT="$(node "${GSD_HOME}/gsd-core/bin/verify-reapply-patches.cjs" "${CLASSIFY_ARGS[@]}")" +INCORPORATED_FILES="$(echo "$CLASSIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));(d.incorporated_files||[]).forEach(f=>process.stdout.write(f+'\n'))")" +``` + +**For every file in `INCORPORATED_FILES`:** + +- **Do NOT merge it. Do NOT re-apply its diff.** Leave the newly installed file exactly as + shipped — the user's customization is already in it. Re-grafting the diff onto a version + that already contains it duplicates the content (both copies run; a stale graft can + contradict a richer native upstream step that replaced it). +- Report the per-file status `Incorporated` — "Already in upstream v{version}" — in the + Step 3 summary and the Step 7 report. +- No Hunk Verification Table rows are required for these files (nothing was merged); the + classifier's structured output is the evidence. If EVERY backed-up file is Incorporated, + still emit the table with a header row and a single note line naming the incorporated + files, so the Step 5b absent-table halt is not tripped. Step 5a passes these files + without special-casing — every user-added line is present in the untouched install. +- This is what ends the re-graft cycle: because the installed file stays identical to what + the release ships, the next update's hash comparison no longer flags it as modified, and + the file drops out of `gsd-local-patches/` on the next cycle. + +Files NOT in `INCORPORATED_FILES` — reported `needs_merge` (with the lines still missing +from the new version) or `unknown` (no confirmable pristine baseline) — take the normal +merge paths below, unchanged. If the classifier cannot run at all (e.g. `GSD_HOME` +unset), fall back to merging every file as before; never report `Incorporated` without the +classifier's structured confirmation. + For each file in `backup-meta.json`: 1. **Read the backed-up version** (user's modified copy from `gsd-local-patches/`) @@ -209,6 +261,9 @@ Compare the three versions to isolate changes: - Sections changed only by upstream → accept upstream version - Sections changed by both → flag as CONFLICT, show both, ask user - Sections unchanged by either → use new version (identical to all three) +- User-added content already present verbatim in the new version → already incorporated + upstream — do NOT re-apply it (the file-level outcome is `Incorporated` when ALL + user-added content is thus present, as determined by the Step 4 pre-flight classifier) ### Two-way merge (fallback when no baseline) @@ -265,7 +320,7 @@ After writing each merged file, verify that user modifications survived the merg 6. **Report status per file:** - `Merged` — user modifications applied cleanly (show summary of what was preserved) - `Conflict` — user reviewed and chose resolution - - `Incorporated` — user's modification was already adopted upstream (only valid when pristine baseline confirms this) + - `Incorporated` — user's modification was already adopted upstream (only valid when pristine baseline confirms this — determined exclusively by the Step 4 pre-flight classifier's `incorporated_files`, never by inspection) **Never report `Skipped — no custom content`.** If a file is in the backup, it has custom content. @@ -439,6 +494,7 @@ Ask user: - [ ] No file classified as "no custom content" or "SKIP" — every backed-up file is definitionally modified - [ ] Three-way merge used when pristine baseline available (git history or gsd-pristine/) - [ ] User modifications identified and merged into new version +- [ ] Superseded customizations classified `Incorporated` by the deterministic pre-flight classifier (pristine-confirmed) and not re-grafted - [ ] Conflicts surfaced to user with both versions shown - [ ] Status reported for each file with summary of what was preserved - [ ] Post-merge verification checks each file for dropped hunks and warns if content appears missing diff --git a/tests/reapply-patches.test.cjs b/tests/reapply-patches.test.cjs index 9f2495d3b..7ca79c6fa 100644 --- a/tests/reapply-patches.test.cjs +++ b/tests/reapply-patches.test.cjs @@ -726,13 +726,17 @@ describe('Bug #2994: reapply-patches workflow references the runtime-installed p }); test('reapply-patches.md references the verifier at gsd-core/bin/verify-reapply-patches.cjs', () => { - const md = fs.readFileSync(REAPPLY_WORKFLOW, 'utf-8'); + const md = fs.readFileSync(REAPPLY_WORKFLOW, 'utf8'); const invocations = extractScriptInvocations(md); const verifierInvocations = invocations.filter(inv => inv.relPath.endsWith('verify-reapply-patches.cjs')); + // #4136: the workflow now invokes the verifier exactly twice — the Step 4 + // pre-flight --classify run (Incorporated detection) and the Step 5a + // post-merge gate run. Both must resolve to the runtime-installed path + // (the sibling test above enforces the path for every invocation). assert.deepEqual( verifierInvocations.map(i => i.relPath), - ['gsd-core/bin/verify-reapply-patches.cjs'], - 'workflow must call the runtime-installed verifier path exactly once', + ['gsd-core/bin/verify-reapply-patches.cjs', 'gsd-core/bin/verify-reapply-patches.cjs'], + 'workflow must call the runtime-installed verifier exactly twice (classify + gate, #4136)', ); }); }); diff --git a/tests/reapply-verify-hunks.test.cjs b/tests/reapply-verify-hunks.test.cjs index 67c719ab1..7aa7ab47c 100644 --- a/tests/reapply-verify-hunks.test.cjs +++ b/tests/reapply-verify-hunks.test.cjs @@ -210,6 +210,8 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => { // this assertion, removing one breaks consumers that switch on the enum. // Bug #3657 added OK_PRISTINE_DRIFT_DETECTED. // Bug #934 added OK_NO_BASELINE. + // Bug #4136 added OK_UNVALIDATED_BASELINE (--classify refuses to confirm + // adoption on an on-disk snapshot with no recorded hash to validate it). assert.deepEqual( Object.keys(REASON).sort(), [ @@ -221,6 +223,7 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => { 'OK_NO_SIGNIFICANT_BACKUP_LINES', 'OK_NO_USER_LINES_VS_PRISTINE', 'OK_PRISTINE_DRIFT_DETECTED', + 'OK_UNVALIDATED_BASELINE', ], ); }); @@ -702,6 +705,7 @@ describe('Bug #3657: pristine-drift does not produce false FAIL_USER_LINES_MISSI * REASON enum shape-lock: the #3657 fix adds OK_PRISTINE_DRIFT_DETECTED. * This assertion locks the updated documented set of stable codes. * Any further additions require updating this assertion. + * Bug #4136 added OK_UNVALIDATED_BASELINE (see the #2969 fold's lock note). */ test('REASON enum includes OK_PRISTINE_DRIFT_DETECTED added by the #3657 fix', () => { assert.deepEqual( @@ -715,6 +719,7 @@ describe('Bug #3657: pristine-drift does not produce false FAIL_USER_LINES_MISSI 'OK_NO_SIGNIFICANT_BACKUP_LINES', 'OK_NO_USER_LINES_VS_PRISTINE', 'OK_PRISTINE_DRIFT_DETECTED', + 'OK_UNVALIDATED_BASELINE', ], ); }); @@ -1172,6 +1177,7 @@ describe('Bug #4086: verifyFile resolves skills entries at the runtime skills ro } + // ──────────────────────────────────────────────────────────────────────── // Folded regression block — #4145 (a hash-matching gsd-pristine/ baseline // stored without the gsd-core/ prefix is never resolved). verifyFile() joined @@ -1439,3 +1445,543 @@ describe('Bug #4145: hash-matching prefix-less pristine baseline is resolved', ( }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// Folded regression block — #4136 (the documented "Incorporated" per-file +// status was unreachable: a customization upstream had adopted was silently +// re-grafted on every future cycle, forever). Adds a --classify pre-merge +// mode to the deterministic verifier: with a hash-validated pristine +// baseline, a file whose EVERY significant user-added line is already +// present verbatim in the freshly installed version is classified +// `incorporated` — the workflow then leaves it untouched (status +// Incorporated, "Already in upstream v{version}") instead of re-grafting. +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:bug-4136-reapply-incorporated-status', () => { +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Bug #4136: reapply-patches.md Step 4 item 6 defines three per-file statuses + * (Merged / Conflict / Incorporated) but no code path computed Incorporated — + * the term existed only in workflow prose, so superseded customizations were + * re-grafted forever (the merged file's hash never re-converged with the + * shipped manifest hash, so saveLocalPatches re-flagged it every update). + * + * Fix: `--classify` mode on the deterministic verifier. Pre-merge, per file: + * - hash-validated pristine + >=1 significant user-added line + every one of + * those lines present verbatim in the fresh install → incorporated + * - hash-validated pristine + some user lines absent → needs_merge + * - anything else (no/mismatched/absent/unvalidated baseline, zero user + * lines, structural failure) → unknown + * + * Incorporated is NEVER produced without baseline confirmation — the issue's + * law that a false Incorporated is worse than none. Per CONTRIBUTING's typed- + * surface standard, assertions go against the frozen CLASSIFICATION enum and + * the structured --json report; zero text matching on human output. + */ + +const { test, describe, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const crypto = require('node:crypto'); +const os = require('node:os'); +const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); + +const ROOT = path.join(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'gsd-core', 'bin', 'verify-reapply-patches.cjs'); +const { REASON, CLASSIFICATION } = require(SCRIPT); + +let tmpRoot; +let patchesDir; +let configDir; +let pristineDir; + +function sha256(content) { + return crypto.createHash('sha256').update(content, 'utf8').digest('hex'); +} + +function writeFile(absPath, content) { + fs.mkdirSync(path.dirname(absPath), { recursive: true }); + fs.writeFileSync(absPath, content); +} + +function writeBackupMeta(pristine_hashes) { + writeFile(path.join(patchesDir, 'backup-meta.json'), JSON.stringify({ pristine_hashes }, null, 2)); +} + +function resetFixture() { + for (const dir of [patchesDir, configDir, pristineDir]) { + cleanup(dir); + } + fs.mkdirSync(patchesDir); + fs.mkdirSync(configDir); + fs.mkdirSync(pristineDir); +} + +/** Runs the verifier in --classify mode with --json. Returns { status, report }. */ +function runClassifier({ includePristine = true } = {}) { + const args = [ + SCRIPT, + '--patches-dir', patchesDir, + '--config-dir', configDir, + ...(includePristine ? ['--pristine-dir', pristineDir] : []), + '--classify', + '--json', + ]; + const r = runNode(args, { timeoutMs: VERIFIER_TIMEOUT_MS }); + return { + status: r.exitCode, + report: r.stdout && r.stdout.length ? JSON.parse(r.stdout) : null, + }; +} + +/** Runs the verifier in default post-merge gate mode with --json. */ +function runGate({ includePristine = true } = {}) { + const args = [ + SCRIPT, + '--patches-dir', patchesDir, + '--config-dir', configDir, + ...(includePristine ? ['--pristine-dir', pristineDir] : []), + '--json', + ]; + const r = runNode(args, { timeoutMs: VERIFIER_TIMEOUT_MS }); + return { + status: r.exitCode, + report: r.stdout && r.stdout.length ? JSON.parse(r.stdout) : null, + }; +} + +before(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4136-')); + patchesDir = path.join(tmpRoot, 'patches'); + configDir = path.join(tmpRoot, 'installed'); + pristineDir = path.join(tmpRoot, 'pristine'); + resetFixture(); +}); + +after(() => { + cleanup(tmpRoot); +}); + +describe('Bug #4136: deterministic Incorporated classification (--classify)', () => { + test('CLASSIFICATION enum exposes the documented set of stable codes', () => { + assert.deepEqual( + Object.keys(CLASSIFICATION).sort(), + ['INCORPORATED', 'NEEDS_MERGE', 'UNKNOWN'], + ); + }); + + test('Row 1 (RED regression): all user-added lines already upstream → incorporated', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/execute-phase.md'; + const pristineContent = [ + 'stock line one that is long enough to be significant', + 'stock line two that is long enough to be significant', + ].join('\n') + '\n'; + const userLine = 'user custom verification gate that upstream adopted verbatim'; + const backupContent = pristineContent + userLine + '\n'; + // Fresh install: upstream shipped the user's line PLUS its own new line. + const freshInstall = pristineContent + userLine + '\n' + + 'brand-new unrelated upstream line shipped in this release\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runClassifier(); + assert.equal(status, 0, `classify must exit 0; report=${JSON.stringify(report)}`); + assert.equal(report.incorporated, 1); + assert.equal(report.incorporated_files.length, 1); + assert.equal(report.incorporated_files[0].replace(/\\/g, '/'), FILE); + const r0 = report.results[0]; + assert.equal(r0.file.replace(/\\/g, '/'), FILE); + assert.equal(r0.classification, CLASSIFICATION.INCORPORATED); + assert.deepEqual(r0.missing, []); + }); + + test('Row 2: user line absent from fresh install → needs_merge; gate still catches drops', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/plan-phase.md'; + const pristineContent = 'stock baseline line long enough to be significant here\n'; + const userLine = 'user custom instruction that upstream did NOT adopt yet'; + const backupContent = pristineContent + userLine + '\n'; + const freshInstall = pristineContent + 'unrelated upstream line shipped in the release\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const cls = runClassifier(); + assert.equal(cls.status, 0, 'classify is informational — needs_merge is not an error'); + assert.equal(cls.report.incorporated, 0); + assert.deepEqual(cls.report.incorporated_files, []); + const c0 = cls.report.results[0]; + assert.equal(c0.classification, CLASSIFICATION.NEEDS_MERGE); + assert.ok(c0.missing.includes(userLine), `missing must name the absent line; got ${JSON.stringify(c0.missing)}`); + + // Negative proof: the post-merge gate is NOT weakened — a merge that + // drops the user line still fails the default (#2969) run. + const gate = runGate(); + assert.equal(gate.status, 1); + assert.equal(gate.report.failures, 1); + const g0 = gate.report.results[0]; + assert.equal(g0.status, 'fail'); + assert.equal(g0.reason, REASON.FAIL_USER_LINES_MISSING); + }); + + test('Row 3 (boundary): partially-superseded is needs_merge, not Incorporated', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/ship.md'; + const pristineContent = 'stock line that is long enough to be significant\n'; + const adoptedLine = 'user line number one that upstream did adopt upstream'; + const unadoptedLine = 'user line number two that upstream has NOT adopted'; + const backupContent = pristineContent + adoptedLine + '\n' + unadoptedLine + '\n'; + const freshInstall = pristineContent + adoptedLine + '\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runClassifier(); + assert.equal(status, 0); + assert.equal(report.incorporated, 0, 'partial adoption must not classify Incorporated'); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.NEEDS_MERGE); + assert.deepEqual(r0.missing, [unadoptedLine], 'only the un-adopted line is missing'); + }); + + test('Row 4a: pristine drift (#3657 shape) → unknown, never Incorporated', () => { + resetFixture(); + const FILE = 'agents/gsd-executor.md'; + const oldPristine = 'old pristine line present when the backup was captured\n'; + const newPristine = 'refreshed upstream snapshot line in the newer GSD release\n'; + const userLine = 'user customisation line that upstream adopted in the release'; + const backupContent = oldPristine + userLine + '\n'; + const freshInstall = newPristine + userLine + '\n'; + + writeBackupMeta({ [FILE]: sha256(oldPristine) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), newPristine); // hash mismatch → drift + + const { status, report } = runClassifier(); + assert.equal(status, 0); + assert.equal(report.incorporated, 0, 'a drifted baseline confirms nothing'); + assert.deepEqual(report.incorporated_files, []); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.UNKNOWN); + assert.equal(r0.reason, REASON.OK_PRISTINE_DRIFT_DETECTED); + }); + + test('Row 4b: recorded hash but pristine absent (#934 shape) → unknown', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/debug.md'; + const pristineContent = 'stock line that is long enough to be significant x\n'; + const userLine = 'user custom line upstream adopted, but baseline is missing'; + const backupContent = pristineContent + userLine + '\n'; + const freshInstall = pristineContent + userLine + '\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + // No pristine file on disk at all. + + const { status, report } = runClassifier(); + assert.equal(status, 0); + assert.equal(report.incorporated, 0, 'absent baseline confirms nothing even when all lines are present'); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.UNKNOWN); + assert.equal(r0.reason, REASON.OK_NO_BASELINE); + }); + + test('Row 4c: no --pristine-dir (two-way fallback) → unknown', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/scan.md'; + const pristineContent = 'stock line that is long enough to be significant y\n'; + const userLine = 'user custom line upstream adopted, but no baseline was passed'; + const backupContent = pristineContent + userLine + '\n'; + const freshInstall = pristineContent + userLine + '\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runClassifier({ includePristine: false }); + assert.equal(status, 0); + assert.equal(report.incorporated, 0); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.UNKNOWN); + }); + + test('Row 4d: on-disk pristine but no recorded hash → unknown (unvalidated baseline)', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/undo.md'; + const pristineContent = 'stock line that is long enough to be significant z\n'; + const userLine = 'user custom line upstream adopted, but hash was never recorded'; + const backupContent = pristineContent + userLine + '\n'; + const freshInstall = pristineContent + userLine + '\n'; + + // Older installer: no pristine_hashes entry at all. + writeBackupMeta({}); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runClassifier(); + assert.equal(status, 0); + assert.equal(report.incorporated, 0, 'an unvalidated snapshot cannot confirm adoption'); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.UNKNOWN); + assert.equal(r0.reason, REASON.OK_UNVALIDATED_BASELINE); + }); + + test('Row 4d-2: a #4145-recovered (hash-matched orphan) baseline can confirm adoption', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/import.md'; + const pristineContent = 'stock line that is long enough to be significant s\n'; + const userLine = 'user custom line upstream adopted, baseline stored unprefixed'; + const backupContent = pristineContent + userLine + '\n'; + const freshInstall = pristineContent + userLine + '\n'; + + // The pristine snapshot sits WITHOUT the gsd-core/ prefix (an earlier + // release's writer dropped it) — only the #4145 hash scan can find it. + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, 'workflows', 'import.md'), pristineContent); + + const { status, report } = runClassifier(); + assert.equal(status, 0); + // A recovered baseline is hash-confirmed by construction, so it VALIDATES + // and may confirm adoption — the #4136 classifier composes with the + // #4145 recovery instead of treating it as unknown. + assert.equal(report.incorporated, 1); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.INCORPORATED); + assert.deepEqual(r0.missing, []); + }); + + test('Row 4e: signature-looking short/fence lines do not drive the classification', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/health.md'; + const pristineContent = 'stock line that is long enough to be significant w\n'; + // The user hunk: a short heading (under the 12-char significance floor), + // a code fence, and ONE significant line. + const shortHeading = '## My Gate'; + const fence = '```bash'; + const significant = 'the substantive customization body line that matters'; + const backupContent = pristineContent + shortHeading + '\n' + fence + '\n' + significant + '\n'; + // Fresh install happens to contain the short heading (renamed section) + // and plenty of fences — but NOT the significant body line. + const freshInstall = pristineContent + '## My Gate\n' + '```bash\nls -la\n```\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runClassifier(); + assert.equal(status, 0); + assert.equal(report.incorporated, 0, 'trivial-line presence must not fabricate adoption'); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.NEEDS_MERGE); + assert.ok(r0.missing.includes(significant)); + assert.ok(!r0.missing.includes(shortHeading), 'insufficiently-significant lines are excluded'); + assert.ok(!r0.missing.includes(fence), 'structural lines are excluded'); + }); + + test('Row 5: backup with zero significant delta vs validated pristine → unknown, never a skip', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/note.md'; + const pristineContent = 'stock line that is long enough to be significant v\n'; + const backupContent = pristineContent; // degenerate: backed up but identical + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), pristineContent); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { status, report } = runClassifier(); + assert.equal(status, 0); + assert.equal(report.incorporated, 0); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.UNKNOWN); + assert.equal(r0.reason, REASON.OK_NO_USER_LINES_VS_PRISTINE); + }); + + test('Row 6: structural failures classify unknown; classify still exits 0', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/stats.md'; + const pristineContent = 'stock line that is long enough to be significant u\n'; + const userLine = 'user custom line that upstream adopted in this release'; + const backupContent = pristineContent + userLine + '\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(pristineDir, FILE), pristineContent); + // Installed file deliberately absent. + + const { status, report } = runClassifier(); + assert.equal(status, 0, 'classify is informational; the post-merge gate enforces structure'); + const r0 = report.results[0]; + assert.equal(r0.classification, CLASSIFICATION.UNKNOWN); + assert.equal(r0.reason, REASON.FAIL_INSTALLED_MISSING); + }); + + test('Row 7: classify --json report shape is { checked, incorporated, incorporated_files, results }', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/help.md'; + const pristineContent = 'stock line that is long enough to be significant t\n'; + const userLine = 'user custom line that upstream adopted in this release'; + const backupContent = pristineContent + userLine + '\n'; + const freshInstall = pristineContent + userLine + '\n'; + + writeBackupMeta({ [FILE]: sha256(pristineContent) }); + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(configDir, FILE), freshInstall); + writeFile(path.join(pristineDir, FILE), pristineContent); + + const { report } = runClassifier(); + assert.deepEqual(Object.keys(report).sort(), ['checked', 'incorporated', 'incorporated_files', 'results']); + const r0 = report.results[0]; + assert.deepEqual(Object.keys(r0).sort(), ['classification', 'file', 'missing', 'reason']); + assert.equal(typeof r0.file, 'string'); + assert.equal(typeof r0.classification, 'string'); + assert.ok(Array.isArray(r0.missing)); + }); + + test('Row 8: Incorporated ends the re-graft cycle — the file drops out of the next backup', () => { + resetFixture(); + const FILE = 'gsd-core/workflows/inbox.md'; + const pristineV1 = 'stock v1 line that is long enough to be significant\n'; + const userLine = 'user custom loop guard that upstream adopted in v2'; + const backupContent = pristineV1 + userLine + '\n'; + // v2 ships the user's line verbatim plus an unrelated upstream change. + const freshV2 = pristineV1 + userLine + '\n' + 'unrelated upstream improvement line in v2\n'; + + // Update #1: installer backs up the modified file + records the pristine. + writeFile(path.join(patchesDir, FILE), backupContent); + writeFile(path.join(pristineDir, FILE), pristineV1); + writeBackupMeta({ [FILE]: sha256(pristineV1) }); + // The update wipes and installs v2; manifest now hashes v2's shipped bytes. + writeFile(path.join(configDir, FILE), freshV2); + const manifestV2 = { version: '2.0.0', files: { [FILE]: sha256(freshV2) } }; + writeFile(path.join(configDir, 'gsd-file-manifest.json'), JSON.stringify(manifestV2, null, 2)); + + // Pre-flight classification: incorporated → the workflow leaves the file untouched. + const cls = runClassifier(); + assert.equal(cls.status, 0); + assert.equal(cls.report.results[0].classification, CLASSIFICATION.INCORPORATED); + assert.equal( + fs.readFileSync(path.join(configDir, FILE), 'utf8'), + freshV2, + 'incorporated files are NOT re-grafted — installed bytes stay as shipped', + ); + + // Post-merge gate passes on the untouched install (all user lines present). + const gate = runGate(); + assert.equal(gate.status, 0, `gate must pass the untouched install; report=${JSON.stringify(gate.report)}`); + assert.equal(gate.report.failures, 0); + + // Next update cycle: saveLocalPatches detection (manifest hash comparison) + // no longer flags the file — the forever loop is broken. + const stillModified = Object.entries(manifestV2.files) + .filter(([rel, hash]) => sha256(fs.readFileSync(path.join(configDir, rel), 'utf8')) !== hash) + .map(([rel]) => rel); + assert.deepEqual(stillModified, [], 'an incorporated file must drop out of the backup cycle'); + + // Counter-case (the pre-fix behavior): a re-grafted file WOULD be flagged again. + writeFile(path.join(configDir, FILE), freshV2 + userLine + '\n'); + const reGraftedModified = Object.entries(manifestV2.files) + .filter(([rel, hash]) => sha256(fs.readFileSync(path.join(configDir, rel), 'utf8')) !== hash) + .map(([rel]) => rel); + assert.deepEqual(reGraftedModified, [FILE], 'a re-grafted file stays in the backup cycle forever'); + }); +}); + +describe('Bug #4136: workflow consumes the classifier (contract rows)', () => { + const WORKFLOW = path.join(ROOT, 'gsd-core', 'workflows', 'reapply-patches.md'); + + test('Step 4 runs the classifier before merging and gates it on PRISTINE_DIR', () => { + // allow-test-rule: source-text-is-the-product (#4136) + // reapply-patches.md is the installed runtime workflow — its text IS the + // deployed behavioral contract for --reapply, so structural assertions + // against the shipped text are the correct test form here. + const md = fs.readFileSync(WORKFLOW, 'utf8'); + const classifyIdx = md.indexOf('--classify'); + assert.ok(classifyIdx > 0, 'Step 4 must invoke the verifier with --classify'); + const mergeRulesIdx = md.indexOf('### Three-way merge (when baseline is available)'); + assert.ok(mergeRulesIdx > 0); + assert.ok( + classifyIdx < mergeRulesIdx, + 'the classify invocation must precede the Step 4 merge rules (classify before merging)', + ); + // Both invocations must be the runtime-installed path (locked separately + // by the #2994 fold); here we lock that the classify block is bounded by + // the same GSD_HOME-anchored script reference as the Step 5a gate. + const gateIdx = md.indexOf('verify-reapply-patches.cjs', classifyIdx); + assert.ok(gateIdx > classifyIdx, 'the Step 5a gate invocation must still follow the classify block'); + }); + + test('Incorporated files are instructed NOT to be re-grafted', () => { + // allow-test-rule: source-text-is-the-product (#4136) + const md = fs.readFileSync(WORKFLOW, 'utf8'); + const step4Idx = md.indexOf('## Step 4: Merge each file'); + const step5Idx = md.indexOf('## Step 5: Hunk Verification Gate'); + assert.ok(step4Idx > 0 && step5Idx > step4Idx); + const step4 = md.slice(step4Idx, step5Idx); + assert.ok( + step4.includes('INCORPORATED_FILES'), + 'Step 4 must consume the classifier\'s incorporated_files list', + ); + assert.ok( + /do NOT re-apply/i.test(step4), + 'Step 4 must instruct that incorporated files are not re-grafted', + ); + assert.ok( + step4.includes('Already in upstream'), + 'the documented Step 7 Incorporated phrasing must be wired to the classifier output', + ); + }); + + test('three-way merge rules include the already-present-verbatim rule', () => { + // allow-test-rule: source-text-is-the-product (#4136) + const md = fs.readFileSync(WORKFLOW, 'utf8'); + const rulesIdx = md.indexOf('**Merge rules:**'); + assert.ok(rulesIdx > 0); + const rulesBlock = md.slice(rulesIdx, md.indexOf('### Two-way merge')); + assert.ok( + rulesBlock.includes('already present verbatim'), + 'the merge rule set must cover user content upstream already contains', + ); + }); + + test('success_criteria covers the Incorporated / not-re-grafted contract', () => { + // allow-test-rule: source-text-is-the-product (#4136) + const md = fs.readFileSync(WORKFLOW, 'utf8'); + const start = md.indexOf(''); + const end = md.indexOf(''); + assert.ok(start > 0 && end > start); + const block = md.slice(start, end); + assert.ok( + block.includes('Incorporated'), + 'success_criteria must name the Incorporated disposition', + ); + assert.ok( + /not re-grafted/i.test(block), + 'success_criteria must require that superseded customizations are not re-grafted', + ); + }); +}); + }); +} +