From d1b56febcb5cf6ed7e0226efffc11c5aa6205d54 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 20 Apr 2026 18:37:42 -0400 Subject: [PATCH] fix(execute-phase): post-merge deletion audit for bulk file deletions (closes #2384) (#2483) * fix(execute-phase): post-merge deletion audit for bulk file deletions (closes #2384) Two data-loss incidents were caused by worktree merges bringing in bulk file deletions silently. The pre-merge check (HEAD...WT_BRANCH) catches deletions on the worktree branch, but files deleted during the merge itself (e.g., from merge conflict resolution or stale branch state) were not audited post-merge. Adds a post-merge audit immediately after git merge --no-ff succeeds: - Counts files deleted outside .planning/ in the merge commit - If count > 5 and ALLOW_BULK_DELETE!=1: reverts the merge with git reset --hard HEAD~1 and continues to the next worktree - Logs the full file list and an escape-hatch instruction Co-Authored-By: Claude Sonnet 4.6 * fix(test): tighten post-merge deletion audit assertions (CodeRabbit #2483) Replace loose substring checks with exact regex assertions: - assert.match against 'git diff --diff-filter=D --name-only HEAD~1 HEAD' - assert.match against threshold gate + ALLOW_BULK_DELETE override condition - assert.match against git reset --hard HEAD~1 revert - assert.match against MERGE_DEL_COUNT grep -vc for non-.planning count Co-Authored-By: Claude Sonnet 4.6 * fix(inventory): update workflow count to 81 (graduation.md added in #2490) Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- get-shit-done/workflows/execute-phase.md | 15 ++++++ ...ug-2384-post-merge-deletion-audit.test.cjs | 54 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 tests/bug-2384-post-merge-deletion-audit.test.cjs diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 422b116f5..8b637c447 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -623,6 +623,21 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT break } + # Post-merge deletion audit: detect bulk file deletions in merge commit (#2384) + # --diff-filter=D HEAD~1 HEAD shows files deleted by the merge commit itself. + # Exclude .planning/ — orchestrator-owned deletions there are expected (resurrections + # are handled below). Require ALLOW_BULK_DELETE=1 to bypass for intentional large refactors. + MERGE_DEL_COUNT=$(git diff --diff-filter=D --name-only HEAD~1 HEAD 2>/dev/null | grep -vc '^\.planning/' || true) + if [ "$MERGE_DEL_COUNT" -gt 5 ] && [ "${ALLOW_BULK_DELETE:-0}" != "1" ]; then + MERGE_DELETIONS=$(git diff --diff-filter=D --name-only HEAD~1 HEAD 2>/dev/null | grep -v '^\.planning/' || true) + echo "⚠ BLOCKED: Merge of $WT_BRANCH deleted $MERGE_DEL_COUNT files outside .planning/ — reverting to protect repository integrity (#2384)" + echo "$MERGE_DELETIONS" + echo " If these deletions are intentional, re-run with ALLOW_BULK_DELETE=1" + git reset --hard HEAD~1 2>/dev/null || true + rm -f "$STATE_BACKUP" "$ROADMAP_BACKUP" + continue + fi + # Restore orchestrator-owned files (main always wins) if [ -s "$STATE_BACKUP" ]; then cp "$STATE_BACKUP" .planning/STATE.md diff --git a/tests/bug-2384-post-merge-deletion-audit.test.cjs b/tests/bug-2384-post-merge-deletion-audit.test.cjs new file mode 100644 index 000000000..04be56ee0 --- /dev/null +++ b/tests/bug-2384-post-merge-deletion-audit.test.cjs @@ -0,0 +1,54 @@ +'use strict'; + +/** + * Regression test for #2384. + * + * During execute-phase, the orchestrator merges per-plan worktree branches into + * main. The pre-merge deletion check (git diff --diff-filter=D HEAD...WT_BRANCH) + * only catches files deleted on the worktree branch. A post-merge audit is also + * required to catch deletions that made it into the merge commit (e.g., files + * that were in the common ancestor but deleted by the merged worktree) and to + * provide a revert safety net. + */ + +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 — post-merge deletion audit (#2384)', () => { + const content = fs.readFileSync(EXECUTE_PHASE, 'utf-8'); + + test('post-merge deletion audit uses merge-commit diff', () => { + assert.match( + content, + /git diff --diff-filter=D --name-only HEAD~1 HEAD/, + 'execute-phase.md must diff HEAD~1..HEAD with --diff-filter=D for post-merge deletion audit' + ); + }); + + test('post-merge audit includes threshold gate + escape hatch + revert path', () => { + assert.match( + content, + /\[\s*"\$MERGE_DEL_COUNT"\s*-gt\s*5\s*\]\s*&&\s*\[\s*"\$\{ALLOW_BULK_DELETE:-0\}"\s*!=\s*"1"\s*\]/, + 'execute-phase.md must gate on MERGE_DEL_COUNT threshold and ALLOW_BULK_DELETE override' + ); + assert.match( + content, + /git reset --hard HEAD~1/, + 'execute-phase.md must revert the merge commit when bulk deletions are blocked' + ); + }); + + test('post-merge audit computes deletion count outside .planning/', () => { + assert.match( + content, + /MERGE_DEL_COUNT=.*grep -vc '\^\\\.planning\//, + 'execute-phase.md must count non-.planning deletions for the bulk-delete guard' + ); + }); +});