mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
fix(execute-phase): resurrection-detection must check git history before deleting new .planning/ files (#2510)
The guard at the worktree-merge resurrection block was inverting the intended logic: it deleted any .planning/ file absent from PRE_MERGE_FILES, which includes brand-new files (e.g. SUMMARY.md just created by the executor). A genuine resurrection is a file that was previously tracked on main, deliberately removed, and then re-introduced by the merge. Detecting that requires a git history check — not just tree membership. Fix: replace the PRE_MERGE_FILES grep guard with a `git log --follow --diff-filter=D` check that only removes the file if it has a deletion event in main's ancestry. Closes #2501
This commit is contained in:
@@ -649,10 +649,15 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT
|
||||
|
||||
# Detect files deleted on main but re-added by worktree merge
|
||||
# (e.g., archived phase directories that were intentionally removed)
|
||||
# A "resurrected" file must have a deletion event in main's ancestry —
|
||||
# brand-new files (e.g. SUMMARY.md just created by the executor) have no
|
||||
# such history and must NOT be removed (#2501).
|
||||
DELETED_FILES=$(git diff --diff-filter=A --name-only HEAD~1 -- .planning/ 2>/dev/null || true)
|
||||
for RESURRECTED in $DELETED_FILES; do
|
||||
# Check if this file was NOT in main's pre-merge tree
|
||||
if ! echo "$PRE_MERGE_FILES" | grep -qxF "$RESURRECTED"; then
|
||||
# Only delete if this file was previously tracked on main and then
|
||||
# deliberately removed (has a deletion event in git history).
|
||||
WAS_DELETED=$(git log --follow --diff-filter=D --name-only --format="" HEAD~1 -- "$RESURRECTED" 2>/dev/null | grep -c . || true)
|
||||
if [ "${WAS_DELETED:-0}" -gt 0 ]; then
|
||||
git rm -f "$RESURRECTED" 2>/dev/null || true
|
||||
fi
|
||||
done
|
||||
|
||||
90
tests/bug-2501-resurrection-detection.test.cjs
Normal file
90
tests/bug-2501-resurrection-detection.test.cjs
Normal file
@@ -0,0 +1,90 @@
|
||||
/**
|
||||
* Tests for bug #2501: resurrection-detection block in execute-phase.md must
|
||||
* check git history before deleting new .planning/ files.
|
||||
*
|
||||
* Root cause: the original logic deleted ANY .planning/ file that was absent
|
||||
* from PRE_MERGE_FILES, which includes brand-new files (e.g. SUMMARY.md)
|
||||
* that the executor just created. A true "resurrection" is a file that was
|
||||
* previously tracked on main, deliberately deleted, and then re-introduced by
|
||||
* a worktree merge. Detecting that requires a git history check, not just a
|
||||
* pre-merge tree membership check.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const EXECUTE_PHASE = path.join(
|
||||
__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md'
|
||||
);
|
||||
|
||||
describe('execute-phase.md — resurrection-detection guard (#2501)', () => {
|
||||
let content;
|
||||
|
||||
// Load once; each test reads from the cached string.
|
||||
test('file is readable', () => {
|
||||
content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
|
||||
assert.ok(content.length > 0, 'execute-phase.md must not be empty');
|
||||
});
|
||||
|
||||
test('resurrection block checks git history for a prior deletion event', () => {
|
||||
if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
|
||||
// Scope check to the resurrection block only (up to 1200 chars from its heading).
|
||||
const resurrectionStart = content.indexOf('# Detect files deleted on main');
|
||||
assert.ok(resurrectionStart !== -1, 'resurrection comment must exist');
|
||||
const window = content.slice(resurrectionStart, resurrectionStart + 1200);
|
||||
|
||||
// The fix must add a git log --diff-filter=D check inside this block so that
|
||||
// only files with a deletion event in the main branch ancestry are removed.
|
||||
const hasHistoryCheck =
|
||||
window.includes('--diff-filter=D') &&
|
||||
window.includes('git log');
|
||||
assert.ok(
|
||||
hasHistoryCheck,
|
||||
'execute-phase.md resurrection block must use "git log ... --diff-filter=D" to verify a file was previously deleted before removing it'
|
||||
);
|
||||
});
|
||||
|
||||
test('resurrection block does not delete files solely because they are absent from PRE_MERGE_FILES', () => {
|
||||
if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
|
||||
// Extract the resurrection section (between the "Detect files deleted on main"
|
||||
// comment and the next empty line / next major comment block).
|
||||
const resurrectionStart = content.indexOf('# Detect files deleted on main');
|
||||
assert.ok(
|
||||
resurrectionStart !== -1,
|
||||
'execute-phase.md must contain the resurrection-detection comment block'
|
||||
);
|
||||
|
||||
// Grab a window of text around the resurrection block (up to 1200 chars).
|
||||
const window = content.slice(resurrectionStart, resurrectionStart + 1200);
|
||||
|
||||
// The ONLY deletion guard should be the history check.
|
||||
// The buggy pattern: `if ! echo "$PRE_MERGE_FILES" | grep -qxF "$RESURRECTED"`
|
||||
// with NO accompanying history check. After the fix the sole condition
|
||||
// determining deletion must involve a git-log history lookup.
|
||||
const hasBuggyStandaloneGuard =
|
||||
/if\s*!\s*echo\s*"\$PRE_MERGE_FILES"\s*\|\s*grep\s+-qxF\s*"\$RESURRECTED"/.test(window) &&
|
||||
!/git log/.test(window);
|
||||
|
||||
assert.ok(
|
||||
!hasBuggyStandaloneGuard,
|
||||
'resurrection block must NOT delete files based solely on absence from PRE_MERGE_FILES without a git-history check'
|
||||
);
|
||||
});
|
||||
|
||||
test('resurrection block still removes files that have a deletion history on main', () => {
|
||||
if (!content) content = fs.readFileSync(EXECUTE_PHASE, 'utf-8');
|
||||
// The fix must still call `git rm` for genuine resurrections.
|
||||
const resurrectionStart = content.indexOf('# Detect files deleted on main');
|
||||
assert.ok(resurrectionStart !== -1, 'resurrection comment must exist');
|
||||
|
||||
const window = content.slice(resurrectionStart, resurrectionStart + 1200);
|
||||
assert.ok(
|
||||
window.includes('git rm'),
|
||||
'resurrection block must still call git rm to remove genuinely resurrected files'
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user