fix(quick): port history-based resurrection guard from execute-phase.md (#3195) (#3201)

Replace the inverted PRE_MERGE_FILES grep in the worktree-merge cleanup
block with the git-log --diff-filter=D history check introduced for
execute-phase.md by PR #2510. The old form deleted any .planning/ file
absent from the pre-merge snapshot — including brand-new files such as
SUMMARY.md — rather than only files with a confirmed deletion event on
main. Remove the now-unused PRE_MERGE_FILES snapshot line. Adds a
drift-guard test (node:test) asserting both workflows use WAS_DELETED and
neither uses the bare PRE_MERGE_FILES grep form.

Closes #3195

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-06 21:51:32 -04:00
committed by GitHub
parent e9a55b4794
commit a6beac40a2
4 changed files with 89 additions and 5 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3195
---
**`/gsd-quick` worktree-merge resurrection guard no longer deletes brand-new `.planning/` files (#3195)** — the inverted `PRE_MERGE_FILES` grep that caused any file absent from the pre-merge snapshot (including freshly created `SUMMARY.md`) to be deleted has been replaced with the git-history check already used by `execute-phase.md` since PR #2510; only files with a confirmed deletion event in main's ancestry are now removed.

View File

@@ -8,6 +8,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Fixed
- **`/gsd-quick` worktree-merge resurrection guard no longer deletes brand-new `.planning/` files** — the inverted `PRE_MERGE_FILES` grep that caused any file absent from the pre-merge snapshot (including freshly created `SUMMARY.md`) to be deleted has been replaced with the git-history check already used by `execute-phase.md` since PR #2510; only files with a confirmed deletion event in main's ancestry are now removed. (#3195)
- **Milestone-archive layout support** — `validate consistency`, `validate health`, and `find-phase` now scan `.planning/milestones/v*-phases/` directories in addition to the flat `.planning/phases/` layout. Projects that have graduated to milestone-archive layout no longer receive spurious W006 "Phase N in ROADMAP.md but no directory on disk" warnings for every active phase. (#3164)
### Feature

View File

@@ -785,9 +785,6 @@ After executor returns:
[ -f .planning/STATE.md ] && cp .planning/STATE.md "$STATE_BACKUP" || true
[ -f .planning/ROADMAP.md ] && cp .planning/ROADMAP.md "$ROADMAP_BACKUP" || true
# Snapshot files on main to detect resurrections
PRE_MERGE_FILES=$(git ls-files .planning/)
# Pre-merge deletion guard: block merges that delete tracked .planning/ files
DELETIONS=$(git diff --diff-filter=D --name-only HEAD..."$WT_BRANCH" 2>/dev/null || true)
if [ -n "$DELETIONS" ]; then
@@ -810,10 +807,17 @@ After executor returns:
if [ -s "$ROADMAP_BACKUP" ]; then cp "$ROADMAP_BACKUP" .planning/ROADMAP.md; fi
rm -f "$STATE_BACKUP" "$ROADMAP_BACKUP"
# Remove files deleted on main but re-added by worktree (--no-ff guarantees a merge commit so HEAD~1 is reliable)
# 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 agent) have no
# such history and must NOT be removed (#2501, #3195).
DELETED_FILES=$(git diff --diff-filter=A --name-only HEAD~1 -- .planning/ 2>/dev/null || true)
for RESURRECTED in $DELETED_FILES; do
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

View File

@@ -0,0 +1,74 @@
/**
* Drift-guard for bug #3195: quick.md and execute-phase.md must both use
* the git-history-based resurrection guard (WAS_DELETED check), not the
* inverted PRE_MERGE_FILES grep form that deletes brand-new files.
*
* The PRE_MERGE_FILES form was fixed in execute-phase.md by PR #2510 but
* the same bug remained in quick.md. This test ensures both workflows stay
* in sync going forward.
*/
'use strict';
const { test, describe } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const QUICK_MD = path.join(
__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'
);
const EXECUTE_PHASE_MD = path.join(
__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md'
);
describe('resurrection guard drift check — quick.md vs execute-phase.md (#3195)', () => {
let quickContent;
let executePhaseContent;
test('both workflow files are readable', () => {
quickContent = fs.readFileSync(QUICK_MD, 'utf-8');
executePhaseContent = fs.readFileSync(EXECUTE_PHASE_MD, 'utf-8');
assert.ok(quickContent.length > 0, 'quick.md must not be empty');
assert.ok(executePhaseContent.length > 0, 'execute-phase.md must not be empty');
});
test('quick.md uses WAS_DELETED (history-check form) in the resurrection block', () => {
if (!quickContent) quickContent = fs.readFileSync(QUICK_MD, 'utf-8');
assert.ok(
quickContent.includes('WAS_DELETED'),
'quick.md must use WAS_DELETED (git log --diff-filter=D history check) in the resurrection guard'
);
});
test('execute-phase.md uses WAS_DELETED (history-check form) in the resurrection block', () => {
if (!executePhaseContent) executePhaseContent = fs.readFileSync(EXECUTE_PHASE_MD, 'utf-8');
assert.ok(
executePhaseContent.includes('WAS_DELETED'),
'execute-phase.md must use WAS_DELETED (git log --diff-filter=D history check) in the resurrection guard'
);
});
test('quick.md does not use the buggy PRE_MERGE_FILES grep form', () => {
if (!quickContent) quickContent = fs.readFileSync(QUICK_MD, 'utf-8');
// The buggy pattern: deletion conditioned on absence from PRE_MERGE_FILES snapshot
const hasBuggyGuard =
quickContent.includes('PRE_MERGE_FILES') &&
/if\s*!\s*echo\s*"\$PRE_MERGE_FILES"\s*\|\s*grep\s+-qxF\s*"\$RESURRECTED"/.test(quickContent);
assert.ok(
!hasBuggyGuard,
'quick.md must NOT delete files based on the PRE_MERGE_FILES snapshot grep (inverted guard bug #3195)'
);
});
test('execute-phase.md does not use the buggy PRE_MERGE_FILES grep form', () => {
if (!executePhaseContent) executePhaseContent = fs.readFileSync(EXECUTE_PHASE_MD, 'utf-8');
const hasBuggyGuard =
executePhaseContent.includes('PRE_MERGE_FILES') &&
/if\s*!\s*echo\s*"\$PRE_MERGE_FILES"\s*\|\s*grep\s+-qxF\s*"\$RESURRECTED"/.test(executePhaseContent);
assert.ok(
!hasBuggyGuard,
'execute-phase.md must NOT delete files based on the PRE_MERGE_FILES snapshot grep (inverted guard bug)'
);
});
});