From dd5d54f182b3272f428fdcde5bcd9b640a043d1c Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 6 Apr 2026 12:20:06 -0700 Subject: [PATCH] enhance(reapply-patches): post-merge verification to catch dropped hunks (#1775) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(reapply-patches): post-merge verification to catch dropped hunks Add a post-merge verification step to the reapply-patches workflow that detects when user-modified content hunks are silently lost during three-way merge. The verification performs line-count sanity checks and hunk-presence verification against signature lines from each user addition. Warnings are advisory — the merge result is kept and the backup remains available for manual recovery. This strengthens the never-skip invariant from PR #1474 by ensuring not just that files are processed, but that their content survives the merge intact. Closes #1758 Co-Authored-By: Claude Opus 4.6 * enhance(reapply-patches): add structural ordering test and refactor test setup (#1758) - Add ordering test: verification section appears between merge-write and status-report steps (positional constraint, not just substring) - Move file reads into before() hook per project test conventions - Update commit prefix from feat: to enhance: per contribution taxonomy (addition to existing workflow, not new concept) Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 --- commands/gsd/reapply-patches.md | 16 +++++ tests/reapply-verify-hunks.test.cjs | 102 ++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 tests/reapply-verify-hunks.test.cjs diff --git a/commands/gsd/reapply-patches.md b/commands/gsd/reapply-patches.md index 1dd2fe012..951050341 100644 --- a/commands/gsd/reapply-patches.md +++ b/commands/gsd/reapply-patches.md @@ -217,6 +217,21 @@ git -C "$CONFIG_DIR" log --oneline --no-merges -- "{file_path}" | grep -v "gsd:u Each matching commit represents an intentional user modification. Use the commit messages and diffs to understand what was changed and why. 4. **Write merged result** to the installed location + +### Post-merge verification + +After writing each merged file, verify that user modifications survived the merge: + +1. **Line-count check:** Count lines in the backup and the merged result. If the merged result has fewer lines than the backup minus the expected upstream removals, flag for review. +2. **Hunk presence check:** For each user-added section identified during diff analysis, search the merged output for at least the first significant line (non-blank, non-comment) of each addition. Missing signature lines indicate a dropped hunk. +3. **Report warnings inline** (do not block): + ``` + ⚠ Potential dropped content in {file_path}: + - Missing hunk near line {N}: "{first_line_preview}..." ({line_count} lines) + - Backup available: {patches_dir}/{file_path} + ``` +4. **Track verification status** — add to per-file report: `Merged (verified)` vs `Merged (⚠ {N} hunks may be missing)` + 5. **Report status per file:** - `Merged` — user modifications applied cleanly (show summary of what was preserved) - `Conflict` — user reviewed and chose resolution @@ -253,4 +268,5 @@ Ask user: - [ ] User modifications identified and merged into new version - [ ] 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-verify-hunks.test.cjs b/tests/reapply-verify-hunks.test.cjs new file mode 100644 index 000000000..4370ab318 --- /dev/null +++ b/tests/reapply-verify-hunks.test.cjs @@ -0,0 +1,102 @@ +/** + * GSD Tools Tests - reapply-patches post-merge verification + * + * Validates that the reapply-patches workflow includes post-merge + * verification to detect dropped hunks during three-way merge. + * + * Closes: #1758 + */ + +const { describe, test, before } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const WORKFLOW_PATH = path.join( + __dirname, '..', 'commands', 'gsd', 'reapply-patches.md' +); + +describe('reapply-patches post-merge verification (#1758)', () => { + let content; + + before(() => { + content = fs.readFileSync(WORKFLOW_PATH, 'utf8'); + }); + + test('workflow file contains "Post-merge verification" section', () => { + assert.ok( + content.includes('Post-merge verification'), + 'workflow must contain a "Post-merge verification" section' + ); + }); + + test('workflow mentions "Hunk presence check"', () => { + + assert.ok( + content.includes('Hunk presence check'), + 'workflow must describe hunk presence checking' + ); + }); + + test('workflow mentions "Line-count check"', () => { + + assert.ok( + content.includes('Line-count check'), + 'workflow must describe line-count sanity checking' + ); + }); + + test('success criteria includes verification', () => { + + const criteria = content.split('')[1] || ''; + assert.ok( + criteria.includes('Post-merge verification') || + criteria.includes('dropped hunks'), + 'success_criteria must reference post-merge verification or dropped hunks' + ); + }); + + test('verification warns but never auto-reverts', () => { + + assert.ok( + content.includes('do not block'), + 'verification must be advisory (do not block)' + ); + }); + + test('verification references backup availability for recovery', () => { + + assert.ok( + content.includes('Backup available'), + 'verification warning must reference backup path for manual recovery' + ); + }); + + test('verification tracks per-file status', () => { + + assert.ok( + content.includes('Merged (verified)') && + content.includes('hunks may be missing'), + 'verification must distinguish "Merged (verified)" from "hunks may be missing" status' + ); + }); + + test('verification section appears between merge-write and status-report steps', () => { + const mergeWritePos = content.indexOf('Write merged result'); + const verificationPos = content.indexOf('Post-merge verification'); + const statusReportPos = content.indexOf('Report status per file'); + + assert.ok(mergeWritePos > -1, 'workflow must contain "Write merged result" step'); + assert.ok(verificationPos > -1, 'workflow must contain "Post-merge verification" section'); + assert.ok(statusReportPos > -1, 'workflow must contain "Report status per file" step'); + + assert.ok( + mergeWritePos < verificationPos, + 'Post-merge verification must appear after "Write merged result"' + ); + assert.ok( + verificationPos < statusReportPos, + 'Post-merge verification must appear before "Report status per file"' + ); + }); +});