* test(#4136): failing-first rows for the unreachable incorporated status Folded block bug-4136-reapply-incorporated-status locks the --classify contract (incorporated / needs_merge / unknown, never-incorporated guards, cycle end-to-end) plus the workflow-contract rows; REASON gains OK_UNVALIDATED_BASELINE in both shape-locks; the #2994 invocation count moves 1 -> 2 (classify + gate). All red until the verifier grows --classify and the workflow consumes it. * fix(#4136): compute the incorporated status; stop re-grafting superseded patches Add --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 (new frozen CLASSIFICATION enum, structured --json report, always exit 0 — the binding gate stays the post-merge run). Drifted (#3657), absent (#934), unvalidated (new OK_UNVALIDATED_BASELINE), and no-baseline runs classify unknown — a false incorporated silently retires a live customization, so only a confirmed baseline may ever confirm adoption. The baseline-resolution block moves out of verifyFile into a shared resolvePristineBaseline so the gate and the classifier cannot drift on what counts as a usable baseline; gate behavior is byte-identical. reapply-patches.md step 4 gains the pre-flight classifier invocation and the not-re-grafted contract: incorporated files are left exactly as shipped (their hash then re-converges with the manifest, ending the backup cycle), statuses feed steps 3/7, and the merge rules gain the already-present-verbatim arm. Emitted-Drift-Ack-Growth: reapply-patches.md — step 4 pre-flight classifier section, the documented Incorporated status needed its deterministic contract * fix(#4136): address review findings on the workflow contract Drop the unused INCORPORATED_COUNT shell variable (standards pass) and close the all-files-incorporated gap in the hunk-table guidance: emit a header row plus a note line so the step 5b absent-table halt is not tripped when nothing was merged (spec pass). Emitted-Drift-Ack-Growth: reapply-patches.md — step 4 pre-flight classifier section, the documented Incorporated status needed its deterministic contract * chore(#4136): backfill changeset fragment with PR 4373 * test(#4136): lock that a 4145-recovered baseline can confirm incorporation The hash-first orphan recovery merged with next (PR #4364) lands in the shared resolvePristineBaseline as a validated resolution; this row pins the composition so a future change cannot quietly downgrade recovered baselines to unknown and silently disable incorporated detection for prefix-less installs. --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/agile-wasps-chatter.md
Normal file
5
.changeset/agile-wasps-chatter.md
Normal file
@@ -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)
|
||||
@@ -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 <path> --config-dir <path> [--pristine-dir <path>] [--json]\n',
|
||||
'usage: verify-reapply-patches.cjs --patches-dir <path> --config-dir <path> [--pristine-dir <path>] [--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,101 +427,31 @@ 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.
|
||||
// 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;
|
||||
}
|
||||
} 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) {
|
||||
// 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) {
|
||||
@@ -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 };
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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('<success_criteria>');
|
||||
const end = md.indexOf('</success_criteria>');
|
||||
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',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user