diff --git a/agents/gsd-executor.md b/agents/gsd-executor.md index cff36f375..82b35a7f6 100644 --- a/agents/gsd-executor.md +++ b/agents/gsd-executor.md @@ -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. diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 0605361ac..043d27cb4 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -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" diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs new file mode 100644 index 000000000..72bf18e8e --- /dev/null +++ b/tests/worktree-safety.test.cjs @@ -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' + ); + }); +});