diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 052bc609..97d2b797 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -485,12 +485,51 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT if [ -n "$WT_BRANCH" ] && [ "$WT_BRANCH" != "HEAD" ]; then CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) + # --- Orchestrator file protection (#1756) --- + # Snapshot orchestrator-owned files BEFORE merge. If the worktree + # branch outlived a milestone transition, its versions of STATE.md + # and ROADMAP.md are stale. Main always wins for these files. + STATE_BACKUP=$(mktemp) + ROADMAP_BACKUP=$(mktemp) + git show HEAD:.planning/STATE.md > "$STATE_BACKUP" 2>/dev/null || true + git show HEAD:.planning/ROADMAP.md > "$ROADMAP_BACKUP" 2>/dev/null || true + + # Snapshot list of files on main BEFORE merge to detect resurrections + PRE_MERGE_FILES=$(git ls-files .planning/) + # 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" + rm -f "$STATE_BACKUP" "$ROADMAP_BACKUP" continue } + # Restore orchestrator-owned files (main always wins) + if [ -s "$STATE_BACKUP" ]; then + cp "$STATE_BACKUP" .planning/STATE.md + fi + if [ -s "$ROADMAP_BACKUP" ]; then + cp "$ROADMAP_BACKUP" .planning/ROADMAP.md + fi + rm -f "$STATE_BACKUP" "$ROADMAP_BACKUP" + + # Detect files deleted on main but re-added by worktree merge + # (e.g., archived phase directories that were intentionally removed) + DELETED_FILES=$(git diff --diff-filter=A --name-only HEAD~1 -- .planning/ 2>/dev/null || true) + for RESURRECTED in $DELETED_FILES; do + # Check if this file was NOT in main's pre-merge tree + if ! echo "$PRE_MERGE_FILES" | grep -qxF "$RESURRECTED"; then + git rm -f "$RESURRECTED" 2>/dev/null || true + fi + done + + # Amend merge commit with restored files if any changed + if ! git diff --quiet .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || \ + [ -n "$DELETED_FILES" ]; then + git add .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || true + git commit --amend --no-edit 2>/dev/null || true + fi + # Remove the worktree git worktree remove "$WT" --force 2>/dev/null || true diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index c3cf36a6..fae32f85 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -603,7 +603,41 @@ After executor returns: for WT in $WORKTREES; do WT_BRANCH=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null) if [ -n "$WT_BRANCH" ] && [ "$WT_BRANCH" != "HEAD" ]; then - git merge "$WT_BRANCH" --no-edit -m "chore: merge quick task worktree ($WT_BRANCH)" 2>&1 || echo "⚠ Merge conflict — resolve manually" + # --- Orchestrator file protection (#1756) --- + # Backup STATE.md and ROADMAP.md before merge (main always wins) + STATE_BACKUP=$(mktemp) + ROADMAP_BACKUP=$(mktemp) + git show HEAD:.planning/STATE.md > "$STATE_BACKUP" 2>/dev/null || true + git show HEAD:.planning/ROADMAP.md > "$ROADMAP_BACKUP" 2>/dev/null || true + + # Snapshot files on main to detect resurrections + PRE_MERGE_FILES=$(git ls-files .planning/) + + git merge "$WT_BRANCH" --no-edit -m "chore: merge quick task worktree ($WT_BRANCH)" 2>&1 || { + echo "⚠ Merge conflict — resolve manually" + rm -f "$STATE_BACKUP" "$ROADMAP_BACKUP" + continue + } + + # Restore orchestrator-owned files + if [ -s "$STATE_BACKUP" ]; then cp "$STATE_BACKUP" .planning/STATE.md; fi + 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 + 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 + git rm -f "$RESURRECTED" 2>/dev/null || true + fi + done + + if ! git diff --quiet .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || \ + [ -n "$DELETED_FILES" ]; then + git add .planning/STATE.md .planning/ROADMAP.md 2>/dev/null || true + git commit --amend --no-edit 2>/dev/null || true + fi + git worktree remove "$WT" --force 2>/dev/null || true git branch -D "$WT_BRANCH" 2>/dev/null || true fi diff --git a/tests/worktree-merge-protection.test.cjs b/tests/worktree-merge-protection.test.cjs new file mode 100644 index 00000000..23c56a46 --- /dev/null +++ b/tests/worktree-merge-protection.test.cjs @@ -0,0 +1,101 @@ +/** + * Worktree merge orchestrator file protection tests + * + * Guards against bug #1756: when a worktree branch outlives a milestone + * transition, git merge silently overwrites STATE.md and ROADMAP.md with + * stale content and resurrects archived phase directories. + * + * Fix: The worktree merge step must backup and restore orchestrator-owned + * files (STATE.md, ROADMAP.md) and detect/remove files that main deleted + * but the worktree branch re-adds. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const EXECUTE_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md'); +const QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); + +describe('worktree merge: orchestrator file protection (#1756)', () => { + test('execute-phase.md backs up STATE.md before worktree merge', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + // The workflow must snapshot STATE.md from main before merging + // to prevent stale worktree content from overwriting it + const mergeIdx = content.indexOf('git merge'); + assert.ok(mergeIdx > -1, 'workflow should contain git merge'); + + // Look for STATE.md backup/snapshot before the merge command + const hasStateBackup = ( + content.includes('STATE.md') && + (content.includes('git show HEAD:.planning/STATE.md') || + content.includes('state-backup') || + content.includes('STATE_BACKUP')) + ); + assert.ok(hasStateBackup, + 'execute-phase must backup STATE.md before worktree merge to prevent stale overwrite'); + }); + + test('execute-phase.md backs up ROADMAP.md before worktree merge', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + + const hasRoadmapBackup = ( + content.includes('ROADMAP.md') && + (content.includes('git show HEAD:.planning/ROADMAP.md') || + content.includes('roadmap-backup') || + content.includes('ROADMAP_BACKUP')) + ); + assert.ok(hasRoadmapBackup, + 'execute-phase must backup ROADMAP.md before worktree merge to prevent stale overwrite'); + }); + + test('execute-phase.md restores orchestrator files after worktree merge', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + + // After merge, orchestrator files must be restored from backup + const mergeIdx = content.indexOf('git merge'); + const restoreSection = content.slice(mergeIdx); + + const hasRestore = ( + restoreSection.includes('cp ') || + restoreSection.includes('git checkout HEAD') || + restoreSection.includes('restore') || + restoreSection.includes('BACKUP') + ); + assert.ok(hasRestore, + 'execute-phase must restore orchestrator files after merge (main always wins)'); + }); + + test('execute-phase.md detects files deleted on main but re-added by worktree', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + + // The merge step should detect and remove resurrected files + // (e.g., archived phase directories that main deleted) + const hasResurrectionDetection = ( + content.includes('git diff') && content.includes('--diff-filter') || + content.includes('resurrect') || + content.includes('re-added') || + content.includes('deleted on main') || + content.includes('DELETED_FILES') || + content.includes('PRE_MERGE_FILES') + ); + assert.ok(hasResurrectionDetection, + 'execute-phase must detect and remove files that main deleted but worktree re-added'); + }); + + test('quick.md has the same orchestrator file protection', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + + const hasProtection = ( + (content.includes('git show HEAD:.planning/STATE.md') || + content.includes('state-backup') || + content.includes('STATE_BACKUP')) && + (content.includes('git show HEAD:.planning/ROADMAP.md') || + content.includes('roadmap-backup') || + content.includes('ROADMAP_BACKUP')) + ); + assert.ok(hasProtection, + 'quick.md must also protect orchestrator files during worktree merge'); + }); +});