enhance(reapply-patches): post-merge verification to catch dropped hunks (#1775)
* 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 <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
</success_criteria>
|
||||
|
||||
102
tests/reapply-verify-hunks.test.cjs
Normal file
102
tests/reapply-verify-hunks.test.cjs
Normal file
@@ -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('<success_criteria>')[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"'
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user