fix(worktree): executor deletion verification and pre-merge deletion block (#2040)

* fix(worktree): use reset --hard in worktree_branch_check to correctly set base (#2015)

The worktree_branch_check in execute-phase.md and quick.md used
git reset --soft as the fallback when EnterWorktree created a branch
from main/master instead of the current feature branch HEAD. --soft
moves the HEAD pointer but leaves working tree files from main unchanged,
so the executor worked against stale code and produced commits containing
the entire feature branch diff as deletions.

Fix: replace git reset --soft with git reset --hard in both workflow files.
--hard resets both the HEAD pointer and the working tree to the expected
base commit. It is safe in a fresh worktree that has no user changes.

Adds 4 regression tests (2 per workflow) verifying that the check uses
--hard and does not contain --soft.

* fix(worktree): executor deletion verification and pre-merge deletion block (#1977)

- Remove Windows-only qualifier from worktree_branch_check in execute-plan.md
  (the EnterWorktree base-branch bug affects all platforms, not just Windows)
- Add post-commit --diff-filter=D deletion check to gsd-executor.md task_commit_protocol
  so unexpected file deletions are flagged immediately after each task commit
- Add pre-merge --diff-filter=D deletion guard to execute-phase.md worktree cleanup
  so worktree branches containing file deletions are blocked before fast-forward merge
- Add regression test tests/worktree-safety.test.cjs covering all three behaviors

Fixes #1977

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-10 12:30:08 -04:00
committed by GitHub
parent fc4fcab676
commit 083b26550b
3 changed files with 132 additions and 1 deletions

View File

@@ -362,7 +362,16 @@ git commit -m "{type}({phase}-{plan}): {concise task description}
- **Single-repo:** `TASK_COMMIT=$(git rev-parse --short HEAD)` — track for SUMMARY.
- **Multi-repo (sub_repos):** Extract hashes from `commit-to-subrepo` JSON output (`repos.{name}.hash`). Record all hashes for SUMMARY (e.g., `backend@abc1234, frontend@def5678`).
**6. Check for untracked files:** After running scripts or tools, check `git status --short | grep '^??'`. For any new untracked files: commit if intentional, add to `.gitignore` if generated/runtime output. Never leave generated files untracked.
**6. Post-commit deletion check:** After recording the hash, verify the commit did not accidentally delete tracked files:
```bash
DELETIONS=$(git diff --diff-filter=D --name-only HEAD~1 HEAD 2>/dev/null || true)
if [ -n "$DELETIONS" ]; then
echo "WARNING: Commit includes file deletions: $DELETIONS"
fi
```
Intentional deletions (e.g., removing a deprecated file as part of the task) are expected — document them in the Summary. Unexpected deletions are a Rule 1 bug: revert and fix before proceeding.
**7. Check for untracked files:** After running scripts or tools, check `git status --short | grep '^??'`. For any new untracked files: commit if intentional, add to `.gitignore` if generated/runtime output. Never leave generated files untracked.
</task_commit_protocol>
<summary_creation>

View File

@@ -510,6 +510,15 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT
# Snapshot list of files on main BEFORE merge to detect resurrections
PRE_MERGE_FILES=$(git ls-files .planning/)
# Pre-merge deletion check: warn if the worktree branch deletes tracked files
DELETIONS=$(git diff --diff-filter=D --name-only HEAD..."$WT_BRANCH" 2>/dev/null || true)
if [ -n "$DELETIONS" ]; then
echo "BLOCKED: Worktree branch $WT_BRANCH contains file deletions: $DELETIONS"
echo "Review these deletions before merging. If intentional, remove this guard and re-run."
rm -f "$STATE_BACKUP" "$ROADMAP_BACKUP"
continue
fi
# Merge the worktree branch into the current branch
git merge "$WT_BRANCH" --no-edit -m "chore: merge executor worktree ($WT_BRANCH)" 2>&1 || {
echo "⚠ Merge conflict from worktree $WT_BRANCH — resolve manually"

View File

@@ -0,0 +1,113 @@
/**
* Worktree commit safety hardening tests (#1977)
*
* Three checks:
* 1. worktree_branch_check in execute-plan.md is NOT labeled as Windows-only
* (the bug affects all platforms — no platform qualifier should narrow the fix)
* 2. gsd-executor.md task_commit_protocol includes post-commit deletion verification
* (using --diff-filter=D to catch accidental file deletions per task)
* 3. execute-phase.md worktree merge section includes pre-merge deletion check
* (using --diff-filter=D to block merges that would delete tracked files)
*/
'use strict';
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const EXECUTE_PLAN_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-plan.md');
const EXECUTOR_AGENT_PATH = path.join(__dirname, '..', 'agents', 'gsd-executor.md');
const EXECUTE_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md');
describe('worktree commit safety hardening (#1977)', () => {
test('execute-plan worktree_branch_check has no Windows-only platform qualifier', () => {
const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8');
// The worktree_branch_check block must exist
assert.ok(
content.includes('worktree_branch_check'),
'execute-plan.md must contain a worktree_branch_check block'
);
// Search the whole file for any Windows-only qualifier near worktree_branch_check
// Must NOT say "Windows-only" or restrict the check to Windows
const hasWindowsOnlyQualifier = (
/Windows.only/i.test(content) ||
/affects Windows only/i.test(content) ||
/only on Windows/i.test(content) ||
/Windows-specific/i.test(content)
);
assert.ok(
!hasWindowsOnlyQualifier,
'worktree_branch_check must not be labeled as Windows-only — the bug affects all platforms'
);
// Must indicate the fix is universal (affects all platforms or similar)
// The description must exist somewhere in the file
const isUniversal = (
/affects all platforms/i.test(content) ||
/all platforms/i.test(content) ||
/cross.platform/i.test(content)
);
assert.ok(
isUniversal,
'worktree_branch_check description must indicate the fix applies to all platforms'
);
});
test('gsd-executor.md task_commit_protocol includes post-commit deletion verification', () => {
const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8');
// Must contain --diff-filter=D deletion check
assert.ok(
content.includes('--diff-filter=D'),
'gsd-executor.md must include --diff-filter=D deletion verification after each task commit'
);
// Must include a WARNING or notice about deletions
assert.ok(
content.includes('WARNING') || content.includes('DELETIONS'),
'gsd-executor.md must warn when a commit includes file deletions'
);
});
test('execute-phase.md worktree merge section includes pre-merge deletion check', () => {
const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8');
// The merge section must exist
const mergeIdx = content.indexOf('git merge');
assert.ok(mergeIdx > -1, 'execute-phase.md must contain a git merge operation');
// Find the window before the merge command to check for pre-merge deletion detection
// Look broadly for --diff-filter=D in the worktree cleanup section
const worktreeCleanupStart = content.indexOf('Worktree cleanup');
assert.ok(
worktreeCleanupStart > -1,
'execute-phase.md must have a worktree cleanup section'
);
const cleanupSection = content.slice(worktreeCleanupStart);
// Must include --diff-filter=D for deletion detection
assert.ok(
cleanupSection.includes('--diff-filter=D'),
'execute-phase.md worktree merge section must include --diff-filter=D to check for deletions before merge'
);
// The deletion check must appear BEFORE the git merge call within the cleanup section
const deletionCheckIdx = cleanupSection.indexOf('--diff-filter=D');
const gitMergeIdx = cleanupSection.indexOf('git merge');
assert.ok(
deletionCheckIdx < gitMergeIdx,
'deletion check (--diff-filter=D) must appear before git merge in the worktree cleanup section'
);
// Must have a BLOCKED or warning message for when deletions are found
assert.ok(
cleanupSection.includes('BLOCKED') || cleanupSection.includes('DELETIONS') || cleanupSection.includes('deletion'),
'execute-phase.md must warn or block when the worktree branch contains file deletions'
);
});
});