From a6beac40a2f20c87153bf0bc84c7d939f0fa1592 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 6 May 2026 21:51:32 -0400 Subject: [PATCH] fix(quick): port history-based resurrection guard from execute-phase.md (#3195) (#3201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/3195-quick-resurrection-guard.md | 5 ++ CHANGELOG.md | 1 + get-shit-done/workflows/quick.md | 14 ++-- ...bug-3195-quick-resurrection-guard.test.cjs | 74 +++++++++++++++++++ 4 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 .changeset/3195-quick-resurrection-guard.md create mode 100644 tests/bug-3195-quick-resurrection-guard.test.cjs diff --git a/.changeset/3195-quick-resurrection-guard.md b/.changeset/3195-quick-resurrection-guard.md new file mode 100644 index 000000000..09dc29f55 --- /dev/null +++ b/.changeset/3195-quick-resurrection-guard.md @@ -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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4b6c029..eb4fa42ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index ead62d44a..9c3c60be5 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -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 diff --git a/tests/bug-3195-quick-resurrection-guard.test.cjs b/tests/bug-3195-quick-resurrection-guard.test.cjs new file mode 100644 index 000000000..2a52adcfe --- /dev/null +++ b/tests/bug-3195-quick-resurrection-guard.test.cjs @@ -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)' + ); + }); +});