mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
fix(workflow): add gated hunk verification table to reapply-patches — structural enforcement of post-merge checks (#2037)
Adds a mandatory Hunk Verification Table output to Step 4 (columns: file, hunk_id, signature_line, line_count, verified) and a new Step 5 gate that STOPs with an actionable error if any row shows verified: no or the table is absent. Prevents the LLM from silently bypassing post-merge checks by making the next step structurally dependent on the table's presence and content. Adds four regression tests covering table presence, column requirements, Step 5 reference, and the gate condition. Fixes #1999 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -230,22 +230,61 @@ After writing each merged file, verify that user modifications survived the merg
|
||||
- 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)`
|
||||
4. **Produce a Hunk Verification Table** — one row per hunk per file. This table is **mandatory output** and must be produced before Step 5 can proceed. Format:
|
||||
|
||||
5. **Report status per file:**
|
||||
| file | hunk_id | signature_line | line_count | verified |
|
||||
|------|---------|----------------|------------|----------|
|
||||
| {file_path} | {N} | {first_significant_line} | {count} | yes |
|
||||
| {file_path} | {N} | {first_significant_line} | {count} | no |
|
||||
|
||||
- `hunk_id` — sequential integer per file (1, 2, 3…)
|
||||
- `signature_line` — first non-blank, non-comment line of the user-added section
|
||||
- `line_count` — total lines in the hunk
|
||||
- `verified` — `yes` if the signature_line is present in the merged output, `no` otherwise
|
||||
|
||||
5. **Track verification status** — add to per-file report: `Merged (verified)` vs `Merged (⚠ {N} hunks may be missing)`
|
||||
|
||||
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)
|
||||
|
||||
**Never report `Skipped — no custom content`.** If a file is in the backup, it has custom content.
|
||||
|
||||
## Step 5: Cleanup option
|
||||
## Step 5: Hunk Verification Gate
|
||||
|
||||
Before proceeding to cleanup, evaluate the Hunk Verification Table produced in Step 4.
|
||||
|
||||
**If the Hunk Verification Table is absent** (Step 4 did not produce it), STOP immediately and report to the user:
|
||||
```
|
||||
ERROR: Hunk Verification Table is missing. Post-merge verification was not completed.
|
||||
Rerun /gsd-reapply-patches to retry with full verification.
|
||||
```
|
||||
|
||||
**If any row in the Hunk Verification Table shows `verified: no`**, STOP and report to the user:
|
||||
```
|
||||
ERROR: {N} hunk(s) failed verification — content may have been dropped during merge.
|
||||
|
||||
Unverified hunks:
|
||||
{file} hunk {hunk_id}: signature line "{signature_line}" not found in merged output
|
||||
|
||||
The backup is preserved at: {patches_dir}/{file}
|
||||
Review the merged file manually, then either:
|
||||
(a) Re-merge the missing content by hand, or
|
||||
(b) Restore from backup: cp {patches_dir}/{file} {installed_path}
|
||||
```
|
||||
|
||||
Do not proceed to cleanup until the user confirms they have resolved all unverified hunks.
|
||||
|
||||
**Only when all rows show `verified: yes`** (or when all files had zero user-added hunks) may execution continue to Step 6.
|
||||
|
||||
## Step 6: Cleanup option
|
||||
|
||||
Ask user:
|
||||
- "Keep patch backups for reference?" → preserve `gsd-local-patches/`
|
||||
- "Clean up patch backups?" → remove `gsd-local-patches/` directory
|
||||
|
||||
## Step 6: Report
|
||||
## Step 7: Report
|
||||
|
||||
```
|
||||
## Patches Reapplied
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
* Closes: #1469
|
||||
*/
|
||||
|
||||
const { test, describe, beforeEach, afterEach } = require('node:test');
|
||||
const { test, describe, before, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
@@ -274,3 +274,75 @@ describe('reapply-patches workflow contract (#1469)', () => {
|
||||
'workflow must describe git-based detection of user changes');
|
||||
});
|
||||
});
|
||||
|
||||
describe('reapply-patches gated hunk verification (#1999)', () => {
|
||||
let workflowContent;
|
||||
|
||||
before(() => {
|
||||
const workflowPath = path.join(__dirname, '..', 'commands', 'gsd', 'reapply-patches.md');
|
||||
workflowContent = fs.readFileSync(workflowPath, 'utf8');
|
||||
});
|
||||
|
||||
test('Step 4 requires a Hunk Verification Table output format', () => {
|
||||
// Step 4 must mandate production of a named table with specific columns
|
||||
assert.ok(
|
||||
workflowContent.includes('Hunk Verification Table'),
|
||||
'Step 4 must require production of a Hunk Verification Table'
|
||||
);
|
||||
});
|
||||
|
||||
test('Hunk Verification Table includes required columns: file, hunk_id, signature_line, line_count, verified', () => {
|
||||
assert.ok(
|
||||
workflowContent.includes('hunk_id'),
|
||||
'Hunk Verification Table must include hunk_id column'
|
||||
);
|
||||
assert.ok(
|
||||
workflowContent.includes('signature_line'),
|
||||
'Hunk Verification Table must include signature_line column'
|
||||
);
|
||||
assert.ok(
|
||||
workflowContent.includes('line_count'),
|
||||
'Hunk Verification Table must include line_count column'
|
||||
);
|
||||
assert.ok(
|
||||
workflowContent.includes('verified'),
|
||||
'Hunk Verification Table must include verified column'
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 5 references the Hunk Verification Table before proceeding', () => {
|
||||
// Step 5 must consume the table produced by Step 4
|
||||
const step5Match = workflowContent.match(/##\s+Step\s+5[^\n]*\n([\s\S]*?)(?=##\s+Step\s+6|<\/process>|$)/);
|
||||
assert.ok(step5Match, 'Step 5 must exist in the workflow');
|
||||
|
||||
const step5Content = step5Match[1];
|
||||
assert.ok(
|
||||
step5Content.includes('Hunk Verification Table'),
|
||||
'Step 5 must reference the Hunk Verification Table'
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 5 includes an explicit gate that stops execution when verification fails', () => {
|
||||
const step5Match = workflowContent.match(/##\s+Step\s+5[^\n]*\n([\s\S]*?)(?=##\s+Step\s+6|<\/process>|$)/);
|
||||
assert.ok(step5Match, 'Step 5 must exist in the workflow');
|
||||
|
||||
const step5Content = step5Match[1];
|
||||
|
||||
// Must refuse to proceed when any hunk is unverified or the table is absent
|
||||
const hasGate = (
|
||||
step5Content.includes('verified: no') ||
|
||||
step5Content.includes('verified: No') ||
|
||||
step5Content.includes('"no"') ||
|
||||
step5Content.includes('STOP')
|
||||
) && (
|
||||
step5Content.includes('absent') ||
|
||||
step5Content.includes('missing') ||
|
||||
step5Content.includes('not present')
|
||||
);
|
||||
|
||||
assert.ok(
|
||||
hasGate,
|
||||
'Step 5 must include an explicit gate: STOP if any row shows verified: no or the table is absent'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user