From 4f56c3b10b861f2cfee1fcde3dbbd67c76742ec3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 20 May 2026 20:56:26 -0400 Subject: [PATCH] =?UTF-8?q?refactor(tests):=20consolidate=20Worktree=20Mod?= =?UTF-8?q?ule=20=E2=80=94=2013=20files=20=E2=86=92=203=20(#3752)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(tests): consolidate Worktree Module — 13 files → 2 Closes #3742 Co-Authored-By: Claude Sonnet 4.6 * fix(changeset): correct frontmatter format for 3742 fragment type:/pr: fields required by docs-lint; replaces @changesets/cli package-bump format with the repo's custom fragment schema. Co-Authored-By: Claude Sonnet 4.6 * refactor(tests): split consolidated worktree.test.cjs along cleanup seam (≤ 800 LOC/file) Co-Authored-By: Claude Sonnet 4.6 * chore(changeset): update 3742 fragment — 13→3 files, ≤800 LOC/file Co-Authored-By: Claude Sonnet 4.6 * chore(changeset): fix pr reference 3738→3752 in 3742 fragment Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/3742-consolidate-worktree-tests.md | 14 + CONTEXT.md | 6 + tests/bug-2015-worktree-base-branch.test.cjs | 80 -- ...2075-worktree-deletion-safeguards.test.cjs | 224 ----- ...ug-2431-worktree-locked-surfacing.test.cjs | 103 --- ...worktree-cleanup-workspace-safety.test.cjs | 349 -------- ...bug-2924-worktree-head-attachment.test.cjs | 463 ----------- tests/bug-3281-worktree-git-timeout.test.cjs | 262 ------ ...ug-3384-worktree-cleanup-manifest.test.cjs | 235 ------ ...bug-3425-worktree-cleanup-cwd-pin.test.cjs | 36 - tests/worktree-cleanup.test.cjs | 671 ++++++++++++++- tests/worktree-merge-protection.test.cjs | 106 --- tests/worktree-safety-policy.test.cjs | 276 ------- tests/worktree-safety.test.cjs | 766 +++++++++++++++--- tests/worktree-stagger.test.cjs | 49 -- tests/worktree.test.cjs | 739 +++++++++++++++++ 16 files changed, 2090 insertions(+), 2289 deletions(-) create mode 100644 .changeset/3742-consolidate-worktree-tests.md delete mode 100644 tests/bug-2015-worktree-base-branch.test.cjs delete mode 100644 tests/bug-2075-worktree-deletion-safeguards.test.cjs delete mode 100644 tests/bug-2431-worktree-locked-surfacing.test.cjs delete mode 100644 tests/bug-2774-worktree-cleanup-workspace-safety.test.cjs delete mode 100644 tests/bug-2924-worktree-head-attachment.test.cjs delete mode 100644 tests/bug-3281-worktree-git-timeout.test.cjs delete mode 100644 tests/bug-3384-worktree-cleanup-manifest.test.cjs delete mode 100644 tests/bug-3425-worktree-cleanup-cwd-pin.test.cjs delete mode 100644 tests/worktree-merge-protection.test.cjs delete mode 100644 tests/worktree-safety-policy.test.cjs delete mode 100644 tests/worktree-stagger.test.cjs create mode 100644 tests/worktree.test.cjs diff --git a/.changeset/3742-consolidate-worktree-tests.md b/.changeset/3742-consolidate-worktree-tests.md new file mode 100644 index 000000000..f38210a59 --- /dev/null +++ b/.changeset/3742-consolidate-worktree-tests.md @@ -0,0 +1,14 @@ +--- +type: Changed +pr: 3752 +--- + + +Consolidates the Worktree Module test cluster from 13 files to 3, satisfying the lint-test-file-count allowlist ceiling. + +- Merged 11 bug-fix CJS test files into `tests/worktree.test.cjs` (branch-check/workspace-safety) and `tests/worktree-cleanup.test.cjs` (HEAD-attachment/cleanup, split along cleanup seam, each ≤ 800 LOC) +- Retained `tests/worktree-safety.test.cjs` with safety policy tests merged in +- No `worktree` entry needed in allowlist (3 files ≤ 4 cluster budget) +- Added Worktree Workstream Seam Module glossary entry to `CONTEXT.md` + +Closes #3742 diff --git a/CONTEXT.md b/CONTEXT.md index 4c3610860..220e74bc8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -79,6 +79,12 @@ Shared CJS/SDK Module owning project-root resolution from any starting directory ### Planning Path Projection Module SDK query Module owning projection from project/workstream context to concrete `.planning` paths. Policy precedence is `explicit workstream > env workstream > env project > root`. Invalid workspace context is a validation error at this seam rather than a silent fallback. +### Worktree Safety Policy Module +CJS Module owning worktree lifecycle safety policy for the GSD orchestration layer. Interface: `resolveWorktreeContext(cwd, deps) → WorktreeContext` (linked-worktree root mapping), `parseWorktreePorcelain(output) → WorktreeEntry[]` (porcelain parser, skips detached HEAD), `planWorktreePrune(repoRoot, opts, deps) → PrunePlan` (metadata-prune plan, never destructive by default), `executeWorktreePrunePlan(plan, deps) → PruneResult` (executes prune; degrades gracefully on git timeout), `listLinkedWorktreePaths(repoRoot, deps) → LinkedPathsResult`, `inspectWorktreeHealth(repoRoot, opts, deps) → HealthResult` (orphan + stale detection), `snapshotWorktreeInventory(repoRoot, opts, deps) → InventoryResult`, `planWorktreeWaveCleanup(repoRoot, manifest) → CleanupPlan` (manifest-scoped, fail-closed), `executeWorktreeWaveCleanupPlan(plan, deps) → CleanupResult`. Source of truth: `get-shit-done/bin/lib/worktree-safety.cjs`. Timeout path: all git subprocess calls are bounded; callers receive `ok:false, reason:'git_timed_out'` rather than a thrown exception. Test anchor: `tests/worktree-safety.test.cjs`. + +### Worktree Lifecycle Module +Workflow contract seam covering agent worktree lifecycle orchestration rules embedded in `get-shit-done/workflows/execute-phase.md`, `quick.md`, `execute-plan.md`, and `agents/gsd-executor.md`. Key invariants: `worktree_branch_check` uses `git reset --hard` (not `--soft`); HEAD attachment verified via `git symbolic-ref` before any reset; positive allow-list `^worktree-agent-*` enforced; `git update-ref` on protected refs is prohibited; cleanup is manifest-scoped (`WAVE_WORKTREE_MANIFEST`) not global-discovery-based; worktree spawning is sequential (one `run_in_background` at a time to avoid `config.lock` contention). Test anchor: `tests/worktree.test.cjs`. + ### Worktree Root Resolution Adapter Module Adapter Module owning linked-worktree root mapping and metadata-prune policy (`git worktree prune` non-destructive default) for planning/workstream callers. diff --git a/tests/bug-2015-worktree-base-branch.test.cjs b/tests/bug-2015-worktree-base-branch.test.cjs deleted file mode 100644 index 846991066..000000000 --- a/tests/bug-2015-worktree-base-branch.test.cjs +++ /dev/null @@ -1,80 +0,0 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may -// reclassify some entries as source-text-is-the-product during migration. - -/** - * Regression test for #2015: worktree executor creates branch from master - * instead of the current feature branch HEAD. - * - * The worktree_branch_check in execute-phase.md and quick.md used - * `git reset --soft {EXPECTED_BASE}` as the recovery action when the - * worktree was created from the wrong base. `reset --soft` moves the HEAD - * pointer but leaves the working tree files from main/master unchanged — - * the executor then works against stale code and its commits contain an - * enormous diff (the entire feature branch) as deletions. - * - * Fix: use `git reset --hard {EXPECTED_BASE}` in the worktree_branch_check. - * In a fresh worktree with no user changes, --hard is safe and correct. - */ - -const { describe, test } = 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_branch_check must use reset --hard not reset --soft (#2015)', () => { - - test('execute-phase.md worktree_branch_check does not use reset --soft', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - - // Extract the worktree_branch_check block - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'execute-phase.md must contain a block'); - - const block = blockMatch[1]; - assert.ok( - !block.includes('reset --soft'), - 'worktree_branch_check must not use reset --soft (leaves working tree files unchanged). Use reset --hard instead.' - ); - }); - - test('execute-phase.md worktree_branch_check uses reset --hard for base correction', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'execute-phase.md must contain a block'); - - const block = blockMatch[1]; - assert.ok( - block.includes('reset --hard'), - 'worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree to the expected base' - ); - }); - - test('quick.md worktree_branch_check does not use reset --soft', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'quick.md must contain a block'); - - const block = blockMatch[1]; - assert.ok( - !block.includes('reset --soft'), - 'quick.md worktree_branch_check must not use reset --soft. Use reset --hard instead.' - ); - }); - - test('quick.md worktree_branch_check uses reset --hard for base correction', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok(blockMatch, 'quick.md must contain a block'); - - const block = blockMatch[1]; - assert.ok( - block.includes('reset --hard'), - 'quick.md worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree' - ); - }); -}); diff --git a/tests/bug-2075-worktree-deletion-safeguards.test.cjs b/tests/bug-2075-worktree-deletion-safeguards.test.cjs deleted file mode 100644 index 57c15ebfe..000000000 --- a/tests/bug-2075-worktree-deletion-safeguards.test.cjs +++ /dev/null @@ -1,224 +0,0 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may -// reclassify some entries as source-text-is-the-product during migration. - -/** - * Regression tests for #2075: gsd-executor worktree merge systematically - * deletes prior-wave committed files. - * - * Three failure modes documented in issue #2075: - * - * Failure Mode B (PRIMARY — unaddressed before this fix): - * Executor agent runs `git clean` inside the worktree, removing files - * committed on the feature branch. git clean treats them as "untracked" - * from the worktree's perspective and deletes them. The executor then - * commits only its own deliverables; the subsequent merge brings the - * deletions onto the main branch. - * - * Failure Mode A (partially addressed in PR #1982): - * Worktree created from wrong branch base. Audit all worktree-spawning - * workflows for worktree_branch_check presence. - * - * Failure Mode C: - * Stale content from wrong base overwrites shared files. Covered by - * the --hard reset in the worktree_branch_check. - * - * Defense-in-depth (from #1977): - * Post-commit deletion check: already in gsd-executor.md (--diff-filter=D). - * Pre-merge deletion check: already in execute-phase.md (--diff-filter=D). - */ - -'use strict'; - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -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'); -const QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); -const DIAGNOSE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'diagnose-issues.md'); - -describe('bug-2075: worktree deletion safeguards', () => { - - describe('Failure Mode B: git clean prohibition in executor agent', () => { - test('gsd-executor.md explicitly prohibits git clean in worktree context', () => { - const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); - - // Must have an explicit prohibition section mentioning git clean - const prohibitsGitClean = ( - content.includes('git clean') && - ( - /NEVER.*git clean/i.test(content) || - /git clean.*NEVER/i.test(content) || - /do not.*git clean/i.test(content) || - /git clean.*prohibited/i.test(content) || - /prohibited.*git clean/i.test(content) || - /forbidden.*git clean/i.test(content) || - /git clean.*forbidden/i.test(content) || - /must not.*git clean/i.test(content) || - /git clean.*must not/i.test(content) - ) - ); - - assert.ok( - prohibitsGitClean, - 'gsd-executor.md must explicitly prohibit git clean — running it inside a worktree deletes files committed on the feature branch (#2075 Failure Mode B)' - ); - }); - - test('gsd-executor.md git clean prohibition explains the worktree data-loss risk', () => { - const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); - - // The prohibition must be accompanied by a reason — not just a bare rule - // Look for the word "worktree" near the git clean prohibition - const gitCleanIdx = content.indexOf('git clean'); - assert.ok(gitCleanIdx > -1, 'gsd-executor.md must mention git clean (to prohibit it)'); - - // Extract context around the git clean mention (500 chars either side) - const contextStart = Math.max(0, gitCleanIdx - 500); - const contextEnd = Math.min(content.length, gitCleanIdx + 500); - const context = content.slice(contextStart, contextEnd); - - const hasWorktreeRationale = ( - /worktree/i.test(context) || - /delete/i.test(context) || - /untracked/i.test(context) - ); - - assert.ok( - hasWorktreeRationale, - 'The git clean prohibition in gsd-executor.md must explain why: git clean in a worktree deletes files that appear untracked but are committed on the feature branch' - ); - }); - }); - - describe('Failure Mode A: worktree_branch_check audit across all worktree-spawning workflows', () => { - test('execute-phase.md has worktree_branch_check block with --hard reset', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok( - blockMatch, - 'execute-phase.md must contain a block' - ); - - const block = blockMatch[1]; - assert.ok( - block.includes('reset --hard'), - 'execute-phase.md worktree_branch_check must use git reset --hard (not --soft)' - ); - assert.ok( - !block.includes('reset --soft'), - 'execute-phase.md worktree_branch_check must not use git reset --soft' - ); - }); - - test('quick.md has worktree_branch_check block with --hard reset', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - - const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); - assert.ok( - blockMatch, - 'quick.md must contain a block' - ); - - const block = blockMatch[1]; - assert.ok( - block.includes('reset --hard'), - 'quick.md worktree_branch_check must use git reset --hard (not --soft)' - ); - assert.ok( - !block.includes('reset --soft'), - 'quick.md worktree_branch_check must not use git reset --soft' - ); - }); - - test('diagnose-issues.md has worktree_branch_check instruction for spawned agents', () => { - const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); - - assert.ok( - content.includes('worktree_branch_check'), - 'diagnose-issues.md must include worktree_branch_check instruction for spawned debug agents' - ); - - assert.ok( - content.includes('reset --hard'), - 'diagnose-issues.md worktree_branch_check must instruct agents to use git reset --hard' - ); - }); - }); - - describe('Defense-in-depth: post-commit deletion check (from #1977)', () => { - test('gsd-executor.md task_commit_protocol has post-commit deletion verification', () => { - const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); - - assert.ok( - content.includes('--diff-filter=D'), - 'gsd-executor.md must include --diff-filter=D to detect accidental file deletions after each commit' - ); - - // Must have a warning about unexpected deletions - assert.ok( - content.includes('DELETIONS') || content.includes('WARNING'), - 'gsd-executor.md must emit a warning when a commit includes unexpected file deletions' - ); - }); - }); - - describe('Defense-in-depth: pre-merge deletion check (from #1977)', () => { - test('execute-phase.md worktree merge section has pre-merge deletion check', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - - const worktreeCleanupStart = content.indexOf('Worktree cleanup'); - assert.ok( - worktreeCleanupStart > -1, - 'execute-phase.md must have a worktree cleanup section' - ); - - const cleanupSection = content.slice(worktreeCleanupStart); - - assert.ok( - cleanupSection.includes('--diff-filter=D'), - 'execute-phase.md worktree cleanup must use --diff-filter=D to block deletion-introducing merges' - ); - - // Deletion check must appear before git merge - const deletionCheckIdx = cleanupSection.indexOf('--diff-filter=D'); - const gitMergeIdx = cleanupSection.indexOf('git merge'); - assert.ok( - deletionCheckIdx < gitMergeIdx, - '--diff-filter=D deletion check must appear before git merge in the worktree cleanup section' - ); - - assert.ok( - cleanupSection.includes('BLOCKED') || cleanupSection.includes('deletion'), - 'execute-phase.md must block or warn when the worktree branch contains file deletions' - ); - }); - - test('quick.md worktree merge section has pre-merge deletion check', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - - const mergeIdx = content.indexOf('git merge'); - assert.ok(mergeIdx > -1, 'quick.md must contain a git merge operation'); - - // Find the worktree cleanup block (starts after "Worktree cleanup") - const worktreeCleanupStart = content.indexOf('Worktree cleanup'); - assert.ok( - worktreeCleanupStart > -1, - 'quick.md must have a worktree cleanup section' - ); - - const cleanupSection = content.slice(worktreeCleanupStart); - - assert.ok( - cleanupSection.includes('--diff-filter=D') || cleanupSection.includes('diff-filter'), - 'quick.md worktree cleanup must check for file deletions before merging' - ); - }); - }); - -}); diff --git a/tests/bug-2431-worktree-locked-surfacing.test.cjs b/tests/bug-2431-worktree-locked-surfacing.test.cjs deleted file mode 100644 index 88dd0345f..000000000 --- a/tests/bug-2431-worktree-locked-surfacing.test.cjs +++ /dev/null @@ -1,103 +0,0 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may -// reclassify some entries as source-text-is-the-product during migration. - -/** - * Regression test for #2431: quick.md and execute-phase.md worktree teardown - * silently accumulates locked worktrees via `2>/dev/null || true`. - * - * Fix: replace the silent-fail pattern with a lock-aware block that surfaces - * the error and provides a user-visible recovery message. - */ - -'use strict'; - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -const QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); -const EXECUTE_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md'); - -function assertNoSilentWorktreeRemove(filePath, label) { - const content = fs.readFileSync(filePath, 'utf-8'); - // The old pattern: git worktree remove "$WT" --force 2>/dev/null || true - const silentRemovePattern = /git worktree remove[^\n]*--force\s+2>\/dev\/null\s*\|\|\s*true/; - assert.ok( - !silentRemovePattern.test(content), - `${label}: must not contain "git worktree remove --force 2>/dev/null || true" (silently swallows errors)` - ); -} - -function assertHasLockAwareBlock(filePath, label) { - const content = fs.readFileSync(filePath, 'utf-8'); - // Fix must include: lock-aware detection (checking .git/worktrees/*/locked) - const hasLockCheck = content.includes('.git/worktrees/') && content.includes('locked'); - assert.ok( - hasLockCheck, - `${label}: must include lock-aware detection (.git/worktrees/.../locked check)` - ); -} - -function assertHasWorktreeUnlock(filePath, label) { - const content = fs.readFileSync(filePath, 'utf-8'); - // Fix must include a git worktree unlock attempt - assert.ok( - content.includes('git worktree unlock'), - `${label}: must include "git worktree unlock" retry attempt` - ); -} - -function assertHasUserVisibleWarning(filePath, label) { - const content = fs.readFileSync(filePath, 'utf-8'); - // Fix must print a user-visible warning on residual worktree failure - const hasWarning = content.includes('Residual worktree') || content.includes('manual cleanup'); - assert.ok( - hasWarning, - `${label}: must include user-visible warning when worktree removal fails` - ); -} - -describe('bug-2431: worktree teardown must surface locked-worktree errors', () => { - test('quick.md exists', () => { - assert.ok(fs.existsSync(QUICK_PATH), 'quick.md should exist'); - }); - - test('execute-phase.md exists', () => { - assert.ok(fs.existsSync(EXECUTE_PHASE_PATH), 'execute-phase.md should exist'); - }); - - test('quick.md: no silent worktree remove pattern', () => { - assertNoSilentWorktreeRemove(QUICK_PATH, 'quick.md'); - }); - - test('execute-phase.md: no silent worktree remove pattern', () => { - assertNoSilentWorktreeRemove(EXECUTE_PHASE_PATH, 'execute-phase.md'); - }); - - test('quick.md: has lock-aware detection block', () => { - assertHasLockAwareBlock(QUICK_PATH, 'quick.md'); - }); - - test('execute-phase.md: has lock-aware detection block', () => { - assertHasLockAwareBlock(EXECUTE_PHASE_PATH, 'execute-phase.md'); - }); - - test('quick.md: has git worktree unlock retry', () => { - assertHasWorktreeUnlock(QUICK_PATH, 'quick.md'); - }); - - test('execute-phase.md: has git worktree unlock retry', () => { - assertHasWorktreeUnlock(EXECUTE_PHASE_PATH, 'execute-phase.md'); - }); - - test('quick.md: has user-visible warning on residual worktree', () => { - assertHasUserVisibleWarning(QUICK_PATH, 'quick.md'); - }); - - test('execute-phase.md: has user-visible warning on residual worktree', () => { - assertHasUserVisibleWarning(EXECUTE_PHASE_PATH, 'execute-phase.md'); - }); -}); diff --git a/tests/bug-2774-worktree-cleanup-workspace-safety.test.cjs b/tests/bug-2774-worktree-cleanup-workspace-safety.test.cjs deleted file mode 100644 index ecb03dab2..000000000 --- a/tests/bug-2774-worktree-cleanup-workspace-safety.test.cjs +++ /dev/null @@ -1,349 +0,0 @@ -/** - * Bug #2774 — Worktree cleanup destroys parent workspace .git - * - * The cleanup blocks in execute-phase.md and quick.md previously used an - * EXCLUSION-based filter: - * - * git worktree list --porcelain | grep "^worktree " | grep -v "$(pwd)$" | sed ... - * - * That filter only excludes the literal `$(pwd)`. When a GSD project is itself - * a git worktree of an upstream main repo (the multi-workspace case, including - * the cross-drive Windows case where `git worktree list` reports the registry - * path as e.g. `E:/...` while `$(pwd)` resolves to `C:/...`), every other - * worktree — including the workspace itself — is wiped, taking the - * workspace's `.git` pointer file with it. - * - * The fix is INCLUSION-based: only target paths matching the agent worktree - * convention (`.claude/worktrees/agent-`), the namespace under which Claude - * Code's `isolation="worktree"` always creates executor worktrees. - * - * These tests assert the cleanup block in BOTH workflow files: - * 1. Includes only paths matching `.claude/worktrees/agent-` (positive filter) - * 2. Does NOT rely on `grep -v "$(pwd)$"` as the sole guard (negative filter) - */ - -'use strict'; - -const { describe, test, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const { execSync } = require('child_process'); -const fs = require('fs'); -const path = require('path'); -const os = require('os'); - -const { cleanup } = require('./helpers.cjs'); - -const isWindows = process.platform === 'win32'; - -// The exact discovery pipeline from get-shit-done/workflows/quick.md and -// get-shit-done/workflows/execute-phase.md (line: `WORKTREES=$(git worktree -// list --porcelain | grep "^worktree " | grep "\.claude/worktrees/agent-" | -// sed 's/^worktree //')`). We invoke it as a standalone shell pipeline -// against either real `git worktree list --porcelain` output (in the -// end-to-end case) or piped-in fixture text (in the unit case). -// Note: execSync runs with `shell: '/bin/sh'` by default, which interprets the -// command string directly — no extra `bash -c '...'` wrapper needed. The -// pipeline string below is the verbatim shell from quick.md / execute-phase.md -// (the RHS of the `WORKTREES=$(...)` substitution). -const DISCOVERY_PIPELINE = - 'grep "^worktree " | grep "\\.claude/worktrees/agent-" | sed \'s/^worktree //\''; - -function runDiscoveryAgainstFixture(porcelain) { - const out = execSync(DISCOVERY_PIPELINE, { - input: porcelain, - encoding: 'utf-8', - }); - return out.split('\n').filter((l) => l.length > 0); -} - -function runDiscoveryAgainstRepo(repoCwd) { - const out = execSync( - `git worktree list --porcelain | ${DISCOVERY_PIPELINE}`, - { cwd: repoCwd, encoding: 'utf-8' } - ); - return out.split('\n').filter((l) => l.length > 0); -} - -function makeTempUpstreamRepo(prefix) { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); - execSync('git init -b main', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config user.email "test@test.com"', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config user.name "Test"', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config commit.gpgsign false', { cwd: tmpDir, stdio: 'pipe' }); - fs.writeFileSync(path.join(tmpDir, 'README.md'), '# upstream\n'); - execSync('git add -A', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "initial"', { cwd: tmpDir, stdio: 'pipe' }); - return tmpDir; -} - -describe('bug #2774 — worktree cleanup pipeline must not target the parent workspace', () => { - describe('discovery pipeline (unit)', () => { - test('selects only the agent worktree when workspace itself is a worktree', () => { - // Fixture mirrors the multi-workspace setup: upstream main + sibling - // workspace worktree + agent worktree under workspace's - // `.claude/worktrees/agent-` namespace. - const porcelain = [ - 'worktree /Users/dev/upstream/get-shit-done', - 'HEAD abc123', - 'branch refs/heads/main', - '', - 'worktree /Users/dev/workspaces/feature-x', - 'HEAD def456', - 'branch refs/heads/workspace/feature-x', - '', - 'worktree /Users/dev/workspaces/feature-x/.claude/worktrees/agent-deadbeef', - 'HEAD 789abc', - 'branch refs/heads/worktree-agent-deadbeef', - '', - ].join('\n'); - - const discovered = runDiscoveryAgainstFixture(porcelain); - - assert.deepEqual( - discovered, - ['/Users/dev/workspaces/feature-x/.claude/worktrees/agent-deadbeef'], - 'pipeline must select only the agent-spawned worktree, never the ' + - 'workspace or upstream main repo' - ); - }); - - test('selects nothing when no agent worktrees exist', () => { - const porcelain = [ - 'worktree /Users/dev/upstream/get-shit-done', - 'HEAD abc123', - 'branch refs/heads/main', - '', - 'worktree /Users/dev/workspaces/feature-x', - 'HEAD def456', - 'branch refs/heads/workspace/feature-x', - '', - ].join('\n'); - - const discovered = runDiscoveryAgainstFixture(porcelain); - - assert.deepEqual(discovered, []); - }); - - test('selects multiple agent worktrees and excludes non-agent paths', () => { - const porcelain = [ - 'worktree /repo/main', - 'HEAD a', - 'branch refs/heads/main', - '', - 'worktree /repo/main/.claude/worktrees/agent-aaa', - 'HEAD b', - 'branch refs/heads/agent-aaa', - '', - 'worktree /repo/main/.claude/worktrees/agent-bbb', - 'HEAD c', - 'branch refs/heads/agent-bbb', - '', - 'worktree /repo/main/some-other-dir', - 'HEAD d', - 'branch refs/heads/feature', - '', - ].join('\n'); - - const discovered = runDiscoveryAgainstFixture(porcelain); - - assert.deepEqual(discovered.sort(), [ - '/repo/main/.claude/worktrees/agent-aaa', - '/repo/main/.claude/worktrees/agent-bbb', - ]); - }); - - test('selects agent worktree even when path contains whitespace', () => { - // Regression for CodeRabbit feedback on PR #2778: `for WT in $WORKTREES` - // splits on whitespace and would emit broken half-paths like - // "/Users/dev/My" and "Workspace/.claude/worktrees/agent-xyz". The - // pipeline output itself is line-delimited and preserves the full path — - // the workflow's loop must consume it line-by-line via `while IFS= read`. - const porcelain = [ - 'worktree /Users/dev/My Workspace', - 'HEAD def456', - 'branch refs/heads/workspace/feature-x', - '', - 'worktree /Users/dev/My Workspace/.claude/worktrees/agent-deadbeef', - 'HEAD 789abc', - 'branch refs/heads/worktree-agent-deadbeef', - '', - ].join('\n'); - - const discovered = runDiscoveryAgainstFixture(porcelain); - - assert.deepEqual( - discovered, - ['/Users/dev/My Workspace/.claude/worktrees/agent-deadbeef'], - 'pipeline output must preserve whitespace-bearing agent worktree path on a single line' - ); - }); - - test('while/read loop iterates each whitespace-bearing path exactly once', - { skip: isWindows ? 'POSIX bash process-substitution `< <(...)` under test; not portable to cmd.exe / git-bash variance' : false }, - () => { - // Verify the actual consumer pattern from quick.md / execute-phase.md: - // while IFS= read -r WT; do ...; done < <() - // Counts the lines yielded to the loop body. With the previous - // `for WT in $WORKTREES` form, a path containing one space would yield - // 2 iterations (broken halves). The `while/read` form yields exactly 1. - const porcelain = [ - 'worktree /tmp/has space/.claude/worktrees/agent-aaa', - 'HEAD a', - 'branch refs/heads/agent-aaa', - '', - 'worktree /tmp/two spaces/.claude/worktrees/agent-bbb', - 'HEAD b', - 'branch refs/heads/agent-bbb', - '', - ].join('\n'); - - // Mirror the workflow's loop verbatim. Print one line per iteration with - // a sentinel so we can count and inspect what the loop actually saw. - const script = ` -while IFS= read -r WT; do - [ -z "$WT" ] && continue - printf 'ITER:%s\\n' "$WT" -done < <(${DISCOVERY_PIPELINE}) -`; - // bash needed for process substitution `< <(...)`. - const out = execSync(`bash -c '${script.replace(/'/g, `'\\''`)}'`, { - input: porcelain, - encoding: 'utf-8', - }); - const iterations = out - .split('\n') - .filter((l) => l.startsWith('ITER:')) - .map((l) => l.slice('ITER:'.length)); - - assert.deepEqual( - iterations, - [ - '/tmp/has space/.claude/worktrees/agent-aaa', - '/tmp/two spaces/.claude/worktrees/agent-bbb', - ], - 'while/read loop must yield exactly one iteration per worktree, with whitespace preserved' - ); - }); - }); - - describe('end-to-end against real git worktrees', - { skip: isWindows ? 'POSIX shell discovery pipeline under test + Windows 8.3 short-name (RUNNER~1) vs long-name path mismatch in temp dirs' : false }, - () => { - let upstream; - let workspace; - let agentWorktree; - let workspacesParent; - - beforeEach(() => { - // Build the multi-worktree scenario from #2774: - // upstream/ <- main repo - // workspace/ <- worktree of upstream (the "workspace") - // workspace/.claude/worktrees/agent-XXXX/ <- agent worktree - upstream = makeTempUpstreamRepo('gsd-2774-upstream-'); - - workspacesParent = fs.mkdtempSync( - path.join(os.tmpdir(), 'gsd-2774-workspaces-') - ); - workspace = path.join(workspacesParent, 'feature-x'); - execSync(`git worktree add -b workspace/feature-x "${workspace}"`, { - cwd: upstream, - stdio: 'pipe', - }); - - const agentDir = path.join(workspace, '.claude', 'worktrees'); - fs.mkdirSync(agentDir, { recursive: true }); - agentWorktree = path.join(agentDir, 'agent-deadbeef'); - execSync( - `git worktree add -b worktree-agent-deadbeef "${agentWorktree}"`, - { cwd: upstream, stdio: 'pipe' } - ); - }); - - afterEach(() => { - try { - execSync('git worktree prune', { cwd: upstream, stdio: 'pipe' }); - } catch (_) { - /* ignore */ - } - cleanup(upstream); - cleanup(workspacesParent); - }); - - test('discovery from inside workspace returns only the agent worktree', () => { - const discovered = runDiscoveryAgainstRepo(workspace); - - // Resolve symlinks (macOS /var → /private/var) for stable comparison. - const expected = fs.realpathSync(agentWorktree); - const actual = discovered.map((p) => fs.realpathSync(p)); - - assert.deepEqual( - actual, - [expected], - 'pipeline must list only the agent worktree, not the workspace or upstream' - ); - }); - - test('running cleanup loop on discovered paths preserves workspace .git', () => { - const workspaceGitBefore = fs.readFileSync( - path.join(workspace, '.git'), - 'utf-8' - ); - assert.ok( - fs.existsSync(path.join(upstream, '.git')), - 'precondition: upstream .git must exist' - ); - - const discovered = runDiscoveryAgainstRepo(workspace); - assert.equal( - discovered.length, - 1, - 'precondition: exactly one agent worktree should be discovered' - ); - - // Execute the cleanup behavior end-to-end: `git worktree remove --force` - // each discovered path. This mirrors the workflow's cleanup loop. - for (const wt of discovered) { - execSync(`git worktree remove --force "${wt}"`, { - cwd: workspace, - stdio: 'pipe', - }); - } - - // Agent worktree dir must be gone. - assert.equal( - fs.existsSync(agentWorktree), - false, - 'agent worktree dir should be removed by cleanup' - ); - - // Workspace `.git` pointer file must still exist and be unchanged — - // the regression we are guarding against. - assert.ok( - fs.existsSync(path.join(workspace, '.git')), - 'workspace .git pointer must survive cleanup (regression #2774)' - ); - assert.equal( - fs.readFileSync(path.join(workspace, '.git'), 'utf-8'), - workspaceGitBefore, - 'workspace .git pointer contents must be unchanged' - ); - - // Upstream repo's .git directory must also be intact. - assert.ok( - fs.existsSync(path.join(upstream, '.git')), - 'upstream .git must survive cleanup' - ); - - // Workspace must still be a functional git worktree. - const branch = execSync('git rev-parse --abbrev-ref HEAD', { - cwd: workspace, - encoding: 'utf-8', - }).trim(); - assert.equal( - branch, - 'workspace/feature-x', - 'workspace must still be a functional worktree on its branch' - ); - }); - }); -}); diff --git a/tests/bug-2924-worktree-head-attachment.test.cjs b/tests/bug-2924-worktree-head-attachment.test.cjs deleted file mode 100644 index be89473d0..000000000 --- a/tests/bug-2924-worktree-head-attachment.test.cjs +++ /dev/null @@ -1,463 +0,0 @@ -/** - * Regression tests for #2924: worktree HEAD attaches to a protected branch - * (master/main) so agent commits land there; the workflow then "self-recovers" - * by force-rewinding the protected branch via `git update-ref refs/heads/master`, - * destroying concurrent work in multi-active scenarios. - * - * Fixes asserted by these tests (parsed structurally — not via raw content - * regex/includes — per project test policy): - * - * 1. The block in execute-phase.md and quick.md - * contains a HEAD-attachment assertion (symbolic-ref + protected-branch - * check) that runs BEFORE any `git reset --hard`. - * 2. The parallel-execution prompt in execute-phase.md and execute-plan.md - * no longer mandates `--no-verify` as the default for worktree-mode commits. - * 3. gsd-executor.md prohibits `git update-ref refs/heads/` as a - * "recovery" path and includes a pre-commit HEAD assertion in the task - * commit protocol. - * 4. No workflow file in get-shit-done/workflows/ contains an unconditional - * `git update-ref refs/heads/master` (or main/develop/trunk) call. - */ - -'use strict'; - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -const REPO_ROOT = path.join(__dirname, '..'); -const EXECUTE_PHASE_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'execute-phase.md'); -const EXECUTE_PLAN_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'execute-plan.md'); -const QUICK_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'quick.md'); -const EXECUTOR_AGENT_PATH = path.join(REPO_ROOT, 'agents', 'gsd-executor.md'); -const GIT_INTEGRATION_PATH = path.join(REPO_ROOT, 'get-shit-done', 'references', 'git-integration.md'); - -/** - * Extract the inner body of a named XML-like block (e.g. ...) - * from a markdown document. Returns null when not found. - */ -function extractNamedBlock(markdown, blockName) { - const open = `<${blockName}>`; - const close = ``; - const start = markdown.indexOf(open); - if (start === -1) return null; - const end = markdown.indexOf(close, start + open.length); - if (end === -1) return null; - return markdown.slice(start + open.length, end); -} - -/** - * Extract all fenced code blocks (```...```) from a markdown chunk. - * Returns array of { lang, body } objects. - */ -function extractFencedCodeBlocks(markdown) { - const blocks = []; - const lines = markdown.split('\n'); - let inFence = false; - let fenceLang = ''; - let buffer = []; - for (const line of lines) { - const trimmed = line.trimStart(); - if (trimmed.startsWith('```')) { - if (!inFence) { - inFence = true; - fenceLang = trimmed.slice(3).trim(); - buffer = []; - } else { - blocks.push({ lang: fenceLang, body: buffer.join('\n') }); - inFence = false; - fenceLang = ''; - buffer = []; - } - } else if (inFence) { - buffer.push(line); - } - } - return blocks; -} - -/** - * Tokenize a shell-like script into individual statements (split on `;`, `&&`, `||`, newlines) - * and return commands as arrays of word tokens. Handles `$(cmd ...)` command substitution - * and `VAR=$(cmd ...)` assignments by extracting the inner command. This is intentionally - * simple — adequate for asserting on the presence of well-known git invocations. - */ -function shellStatements(script) { - const statements = []; - const lines = script.split('\n'); - for (let raw of lines) { - const line = raw.replace(/#.*$/, '').trim(); - if (!line) continue; - // Split on shell statement separators - const parts = line.split(/(?:&&|\|\||;)/); - for (const part of parts) { - let trimmed = part.trim(); - if (!trimmed) continue; - // Strip leading `VAR=` assignments so the substituted command surfaces as cmd[0]. - // Then unwrap `$(...)` command substitution. - const assignMatch = trimmed.match(/^[A-Za-z_][A-Za-z0-9_]*=(.*)$/); - if (assignMatch) trimmed = assignMatch[1]; - const subMatch = trimmed.match(/^\$\((.*?)\)?$/); - if (subMatch) trimmed = subMatch[1]; - // Also handle leading `$(` without closing paren (paren may have been split off) - if (trimmed.startsWith('$(')) trimmed = trimmed.slice(2); - // Strip trailing closing parens left over from substitution - trimmed = trimmed.replace(/\)+\s*$/, '').trim(); - if (!trimmed) continue; - // Strip surrounding quotes on the leading word - statements.push(trimmed.split(/\s+/).filter(Boolean)); - } - } - return statements; -} - -/** - * Find the line index of the first command matching a predicate. - * Returns -1 when not found. - */ -function findCommandIndex(statements, predicate) { - for (let i = 0; i < statements.length; i++) { - if (predicate(statements[i])) return i; - } - return -1; -} - -describe('bug #2924: worktree HEAD attachment + destructive recovery', () => { - describe('execute-phase.md worktree_branch_check', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'worktree_branch_check'); - - test('block exists', () => { - assert.ok(block, 'execute-phase.md must contain a block'); - }); - - test('block invokes `git symbolic-ref` to inspect HEAD attachment', () => { - const codeBlocks = extractFencedCodeBlocks(block); - const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); - const idx = findCommandIndex(allStatements, (cmd) => - cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') - ); - assert.notStrictEqual( - idx, -1, - 'worktree_branch_check must run `git symbolic-ref ... HEAD` to verify HEAD attachment before any reset' - ); - }); - - test('HEAD-attachment assertion runs BEFORE `git reset --hard`', () => { - const codeBlocks = extractFencedCodeBlocks(block); - const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); - const symbolicRefIdx = findCommandIndex(allStatements, (cmd) => - cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') - ); - const resetHardIdx = findCommandIndex(allStatements, (cmd) => - cmd[0] === 'git' && cmd[1] === 'reset' && cmd.includes('--hard') - ); - assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref check must exist'); - assert.notStrictEqual(resetHardIdx, -1, 'reset --hard must exist'); - assert.ok( - symbolicRefIdx < resetHardIdx, - 'HEAD attachment assertion (symbolic-ref) must precede `git reset --hard` so a stale HEAD never moves a protected branch' - ); - }); - - test('block names protected branches that must NOT be the agent branch', () => { - // The protected-branch list must be enforced by name. Parse it out of the - // shell scripts and verify required names are present. - const codeBlocks = extractFencedCodeBlocks(block); - const scripts = codeBlocks.map(({ body }) => body).join('\n'); - // Look for an assignment whose value is a regex/list naming protected refs. - // Acceptable forms: PROTECTED_BRANCHES_RE='...' or grep -Eq '^(main|...)$' - // Parse the alternation list out of the grep -E pattern so we assert - // structurally on the protected-branch enumeration rather than via - // raw substring matching (release/* contains regex-special chars and - // can't be safely tested with `\b...\b`). - const altMatch = scripts.match(/grep\s+-Eq?\s+'\^\(([^)]+)\)\$'/); - assert.ok( - altMatch, - 'worktree_branch_check must contain a `grep -Eq` protected-branch alternation pattern' - ); - const branches = altMatch[1].split('|').map((b) => b.trim()); - const required = ['main', 'master', 'develop', 'trunk', 'release/.*']; - for (const name of required) { - assert.ok( - branches.includes(name), - `worktree_branch_check protected-branch alternation must include '${name}' (found: ${branches.join(', ')})` - ); - } - }); - - test('block enforces positive worktree-agent-* allow-list (#2924 hardening)', () => { - const codeBlocks = extractFencedCodeBlocks(block); - const scripts = codeBlocks.map(({ body }) => body).join('\n'); - // Allow-list must reference the canonical Claude Code worktree-agent- - // namespace via a regex assertion (grep -Eq '^worktree-agent-...'). - const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; - assert.ok( - allowListRe.test(scripts), - 'worktree_branch_check must enforce a positive allow-list matching ^worktree-agent-* (#2924 hardening)' - ); - }); - - test('block forbids `git update-ref` self-recovery in its guidance text', () => { - // The forbidding statement is documentation text, not a shell command, - // so structural shell parsing does not apply. Verify the prohibition - // appears as standalone guidance somewhere in the block. - assert.ok( - block.includes('update-ref'), - 'worktree_branch_check must explicitly forbid `git update-ref` self-recovery' - ); - }); - }); - - describe('execute-phase.md no longer defaults to --no-verify in parallel mode', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'parallel_execution'); - - test('parallel_execution block exists', () => { - assert.ok(block, 'execute-phase.md must contain a block'); - }); - - test('parallel_execution does NOT instruct agents to use --no-verify by default', () => { - // Tokenize the block as plain words and look for an unconditional - // imperative naming `--no-verify`. The acceptable presence is in a - // negated/opt-out context (e.g. "Do NOT pass --no-verify"); reject - // any sentence whose first verb is "Use --no-verify". - const sentences = block - .replace(/\n+/g, ' ') - .split(/(?<=[.!?])\s+/); - for (const sentence of sentences) { - if (!sentence.includes('--no-verify')) continue; - const lower = sentence.toLowerCase(); - const isProhibition = - /\b(do not|don't|never|no longer)\b/.test(lower) || - /\bopt[\s-]?out\b/.test(lower) || - /\bopt[\s-]?in\b/.test(lower) || - /\bif\b/.test(lower); - assert.ok( - isProhibition, - `parallel_execution sentence appears to mandate --no-verify by default: "${sentence.trim()}"` - ); - } - }); - }); - - describe('execute-plan.md no longer mandates --no-verify for parallel executor', () => { - const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'precommit_failure_handling'); - test('precommit_failure_handling block exists', () => { - assert.ok(block, 'execute-plan.md must contain a block'); - }); - - test('parallel-executor sub-section does not unconditionally mandate --no-verify', () => { - // Locate the parallel-executor sub-section heading and parse the - // sentences under it. - const headingIdx = block.indexOf('parallel executor'); - assert.notStrictEqual(headingIdx, -1, 'must contain a parallel-executor sub-section'); - const endIdx = block.indexOf('**If running as the sole', headingIdx); - assert.notStrictEqual(endIdx, -1, 'parallel-executor sub-section terminator must exist'); - const subBlock = block.slice(headingIdx, endIdx); - assert.ok(subBlock.length > 0, 'sub-section must have content'); - const sentences = subBlock.replace(/\n+/g, ' ').split(/(?<=[.!?])\s+/); - for (const sentence of sentences) { - if (!sentence.includes('--no-verify')) continue; - const lower = sentence.toLowerCase(); - const isProhibition = - /\b(do not|don't|never|no longer)\b/.test(lower) || - /\bopt[\s-]?out\b/.test(lower) || - /\bopt[\s-]?in\b/.test(lower) || - /\bif\b/.test(lower); - assert.ok( - isProhibition, - `parallel-executor guidance sentence appears to mandate --no-verify: "${sentence.trim()}"` - ); - } - }); - }); - - describe('quick.md worktree_branch_check', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'worktree_branch_check'); - - test('block exists', () => { - assert.ok(block, 'quick.md must contain a block'); - }); - - test('block references `git symbolic-ref` for HEAD attachment assertion', () => { - // quick.md uses inline `git symbolic-ref ... HEAD` rather than a fenced - // block, so search the block as a token stream of statements. - const statements = shellStatements(block); - const idx = findCommandIndex(statements, (cmd) => - cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') - ); - assert.notStrictEqual( - idx, -1, - 'quick.md worktree_branch_check must run `git symbolic-ref ... HEAD`' - ); - }); - - test('HEAD assertion precedes `git reset --hard`', () => { - const symbolicRefByteIdx = block.indexOf('symbolic-ref'); - const resetHardByteIdx = block.indexOf('reset --hard'); - assert.notStrictEqual(symbolicRefByteIdx, -1); - assert.notStrictEqual(resetHardByteIdx, -1); - assert.ok( - symbolicRefByteIdx < resetHardByteIdx, - 'symbolic-ref HEAD assertion must appear before `git reset --hard` in quick.md worktree_branch_check' - ); - }); - - test('block forbids `git update-ref` self-recovery', () => { - assert.ok( - block.includes('update-ref'), - 'quick.md worktree_branch_check must explicitly forbid `git update-ref` self-recovery' - ); - }); - - test('block enforces positive worktree-agent-* allow-list (#2924 hardening)', () => { - const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; - assert.ok( - allowListRe.test(block), - 'quick.md worktree_branch_check must enforce a positive allow-list matching ^worktree-agent-* (#2924 hardening)' - ); - }); - }); - - describe('quick.md pre-dispatch plan commit no longer hard-codes --no-verify', () => { - const content = fs.readFileSync(QUICK_PATH, 'utf-8'); - const codeBlocks = extractFencedCodeBlocks(content); - // Find the bash block containing the pre-dispatch plan commit - const target = codeBlocks.find(({ body }) => - body.includes('pre-dispatch plan') && body.includes('git commit') - ); - test('pre-dispatch plan commit block exists', () => { - assert.ok(target, 'quick.md must contain the pre-dispatch plan commit block'); - }); - - test('pre-dispatch plan commit gates --no-verify behind a config flag', () => { - // The block must contain BOTH a `git commit` without --no-verify AND - // gate any --no-verify variant inside an `if` block reading a config - // value (workflow.worktree_skip_hooks). - const statements = shellStatements(target.body); - const noVerifyCommits = statements.filter((cmd) => - cmd[0] === 'git' && cmd[1] === 'commit' && cmd.includes('--no-verify') - ); - const cleanCommits = statements.filter((cmd) => - cmd[0] === 'git' && cmd[1] === 'commit' && !cmd.includes('--no-verify') - ); - assert.ok( - cleanCommits.length >= 1, - 'must include at least one `git commit` without --no-verify (default path)' - ); - // If --no-verify still appears, the block must reference the opt-in flag. - if (noVerifyCommits.length > 0) { - assert.ok( - target.body.includes('worktree_skip_hooks'), - '--no-verify commits must be gated behind workflow.worktree_skip_hooks config flag' - ); - } - }); - }); - - describe('gsd-executor.md prohibits update-ref self-recovery', () => { - const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'destructive_git_prohibition'); - - test('destructive_git_prohibition block exists', () => { - assert.ok(block, 'gsd-executor.md must contain a block'); - }); - - test('block prohibits `git update-ref refs/heads/`', () => { - assert.ok( - block.includes('update-ref'), - 'destructive_git_prohibition must enumerate `git update-ref` as a prohibited command' - ); - assert.ok( - block.includes('protected') || block.includes('main') || block.includes('master'), - 'destructive_git_prohibition must call out protected branches in the update-ref prohibition' - ); - }); - - test('block references issue #2924', () => { - assert.ok( - block.includes('#2924'), - 'destructive_git_prohibition should cite #2924 as the source of the update-ref prohibition' - ); - }); - }); - - describe('gsd-executor.md task_commit_protocol enforces worktree-agent-* allow-list', () => { - const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); - const block = extractNamedBlock(content, 'task_commit_protocol'); - - test('task_commit_protocol block exists', () => { - assert.ok(block, 'gsd-executor.md must contain a block'); - }); - - test('step 0 enforces positive worktree-agent-* allow-list (#2924 hardening)', () => { - const codeBlocks = extractFencedCodeBlocks(block); - const scripts = codeBlocks.map(({ body }) => body).join('\n'); - const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; - assert.ok( - allowListRe.test(scripts), - 'task_commit_protocol step 0 must enforce a positive allow-list matching ^worktree-agent-* in addition to the protected-ref deny-list (#2924 hardening)' - ); - }); - }); - - describe('no workflow file performs unconditional update-ref on a protected branch', () => { - const workflowsDir = path.join(REPO_ROOT, 'get-shit-done', 'workflows'); - const workflowFiles = fs - .readdirSync(workflowsDir, { recursive: true }) - .filter((f) => typeof f === 'string' && f.endsWith('.md')) - .map((f) => path.join(workflowsDir, f)); - - for (const filePath of workflowFiles) { - test(`${path.basename(filePath)} contains no update-ref of a protected ref`, () => { - const content = fs.readFileSync(filePath, 'utf-8'); - const blocks = extractFencedCodeBlocks(content); - for (const { body } of blocks) { - const statements = shellStatements(body); - for (const cmd of statements) { - if (cmd[0] !== 'git') continue; - if (cmd[1] !== 'update-ref') continue; - // Reject any update-ref that targets a protected ref. - const target = cmd[2] || ''; - const protectedRe = /^refs\/heads\/(main|master|develop|trunk|release\/.+)$/; - assert.ok( - !protectedRe.test(target), - `${path.basename(filePath)} contains forbidden 'git update-ref ${target}' (#2924)` - ); - } - } - }); - } - }); - - describe('git-integration.md guidance reflects new default', () => { - const content = fs.readFileSync(GIT_INTEGRATION_PATH, 'utf-8'); - test('parallel-agents guidance no longer mandates --no-verify', () => { - // Find the parallel-agents callout and parse its sentences. - const idx = content.indexOf('Parallel agents'); - assert.notStrictEqual(idx, -1, 'must contain a "Parallel agents" callout'); - const section = content.slice(idx); - const endMatch = section.slice(1).match(/\n#{1,6}\s/); - assert.ok(endMatch, 'Parallel agents section must terminate at the next heading'); - const tail = section.slice(0, 1 + endMatch.index); - const sentences = tail.replace(/\n+/g, ' ').split(/(?<=[.!?])\s+/); - for (const sentence of sentences) { - if (!sentence.includes('--no-verify')) continue; - const lower = sentence.toLowerCase(); - const isProhibition = - /\b(do not|don't|never|no longer)\b/.test(lower) || - /\bopt[\s-]?out\b/.test(lower) || - /\bopt[\s-]?in\b/.test(lower) || - /\bif\b/.test(lower); - assert.ok( - isProhibition, - `git-integration.md "Parallel agents" sentence appears to mandate --no-verify: "${sentence.trim()}"` - ); - } - }); - }); -}); diff --git a/tests/bug-3281-worktree-git-timeout.test.cjs b/tests/bug-3281-worktree-git-timeout.test.cjs deleted file mode 100644 index 3e3127da2..000000000 --- a/tests/bug-3281-worktree-git-timeout.test.cjs +++ /dev/null @@ -1,262 +0,0 @@ -/** - * Regression tests for #3281: - * Worktree health paths can hang indefinitely due to unbounded git subprocess calls. - * - * Acceptance criteria: - * AC1 — Worktree git subprocess calls use bounded execution (timeout + deterministic failure). - * AC2 — Timeout/failure outcomes produce structured non-fatal warning signals. - * AC3 — validate health and init progress remain non-crashing when git is unavailable/stalled, - * but report degraded worktree health-check status. - * AC4 — Regression tests cover timeout/degraded-git behavior for worktree safety checks. - */ - -'use strict'; - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const path = require('path'); - -// ─── Module paths ───────────────────────────────────────────────────────────── - -const WORKTREE_SAFETY_PATH = path.join( - __dirname, '..', 'get-shit-done', 'bin', 'lib', 'worktree-safety.cjs' -); - -// ─── Shared timeout stub ────────────────────────────────────────────────────── - -/** - * Returns an execGit stub that simulates what spawnSync returns when the - * subprocess is killed by SIGTERM after exceeding its timeout option. - * Per Node.js docs: result.status === null, result.signal === 'SIGTERM', - * result.error?.code === 'ETIMEDOUT'. - * - * The production execGit implementation must detect this shape and: - * - return { ..., timedOut: true } so callers can distinguish timeout from auth failure - * - not throw - */ -function makeTimeoutStub() { - return function stubTimedOutExecGit(_args, _opts) { - return { - exitCode: null, - stdout: '', - stderr: '', - timedOut: true, - signal: 'SIGTERM', - error: Object.assign(new Error('spawnSync git ETIMEDOUT'), { code: 'ETIMEDOUT' }), - }; - }; -} - -// ─── AC1 / AC4: degraded health via exported functions ─────────────────────── - -describe('bug-3281 AC1: worktree functions return degraded-ok on timeout, not throw', () => { - test('planWorktreePrune returns action=skip when execGit times out', () => { - const { planWorktreePrune } = require(WORKTREE_SAFETY_PATH); - - let threw = false; - let result; - try { - result = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); - } catch { - threw = true; - } - - assert.strictEqual(threw, false, 'planWorktreePrune must not throw on timeout'); - assert.strictEqual(typeof result, 'object', 'planWorktreePrune must return an object'); - assert.strictEqual(result.action, 'skip', 'planWorktreePrune must return action=skip when git times out'); - assert.ok( - typeof result.reason === 'string' && result.reason.length > 0, - 'planWorktreePrune must return a non-empty reason when git times out' - ); - }); - - test('executeWorktreePrunePlan returns ok:false when plan is skip (timeout path)', () => { - const { planWorktreePrune, executeWorktreePrunePlan } = require(WORKTREE_SAFETY_PATH); - - const plan = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); - const result = executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() }); - - assert.strictEqual(typeof result, 'object', 'executeWorktreePrunePlan must return an object'); - assert.strictEqual(result.ok, false, 'executeWorktreePrunePlan must return ok:false on timeout'); - }); - - test('inspectWorktreeHealth returns ok:false when git times out', () => { - const { inspectWorktreeHealth } = require(WORKTREE_SAFETY_PATH); - - let threw = false; - let result; - try { - result = inspectWorktreeHealth('/tmp', {}, { execGit: makeTimeoutStub() }); - } catch { - threw = true; - } - - assert.strictEqual(threw, false, 'inspectWorktreeHealth must not throw on timeout'); - assert.strictEqual(typeof result, 'object'); - assert.strictEqual(result.ok, false, 'inspectWorktreeHealth must return ok:false on timeout'); - }); - - test('listLinkedWorktreePaths returns ok:false on timeout, not throw', () => { - const { listLinkedWorktreePaths } = require(WORKTREE_SAFETY_PATH); - - let threw = false; - let result; - try { - result = listLinkedWorktreePaths('/tmp', { execGit: makeTimeoutStub() }); - } catch { - threw = true; - } - - assert.strictEqual(threw, false, 'listLinkedWorktreePaths must not throw on timeout'); - assert.strictEqual(result.ok, false, 'listLinkedWorktreePaths must return ok:false on timeout'); - assert.ok( - typeof result.reason === 'string' && result.reason.length > 0, - 'listLinkedWorktreePaths must return non-empty reason on timeout' - ); - }); - - test('snapshotWorktreeInventory returns ok:false with reason on timeout, not throw', () => { - const { snapshotWorktreeInventory } = require(WORKTREE_SAFETY_PATH); - - let threw = false; - let result; - try { - result = snapshotWorktreeInventory('/tmp', {}, { execGit: makeTimeoutStub() }); - } catch { - threw = true; - } - - assert.strictEqual(threw, false, 'snapshotWorktreeInventory must not throw on timeout'); - assert.strictEqual(typeof result, 'object'); - assert.strictEqual(result.ok, false, 'snapshotWorktreeInventory must return ok:false on timeout'); - assert.ok( - typeof result.reason === 'string' && result.reason.length > 0, - 'snapshotWorktreeInventory must return non-empty reason on timeout' - ); - }); - - test('resolveWorktreeContext returns a valid result on timeout, not throw', () => { - const { resolveWorktreeContext } = require(WORKTREE_SAFETY_PATH); - - let threw = false; - let result; - try { - result = resolveWorktreeContext('/tmp', { execGit: makeTimeoutStub() }); - } catch { - threw = true; - } - - assert.strictEqual(threw, false, 'resolveWorktreeContext must not throw on timeout'); - assert.strictEqual(typeof result, 'object'); - assert.ok( - typeof result.effectiveRoot === 'string', - 'resolveWorktreeContext must return effectiveRoot string even on timeout' - ); - }); -}); - -// ─── AC2 / AC4: timedOut is a first-class field in results ─────────────────── - -describe('bug-3281 AC2+AC4: timedOut is a first-class field in results', () => { - test('planWorktreePrune reason is git_timed_out when execGit returns timedOut:true', () => { - const { planWorktreePrune } = require(WORKTREE_SAFETY_PATH); - - const result = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); - - // AC4 strict: must use the specific reason string 'git_timed_out' - // (not the generic 'git_list_failed') to distinguish timeout from auth failure - assert.strictEqual( - result.reason, - 'git_timed_out', - [ - 'AC4 (strict): planWorktreePrune must use reason=git_timed_out', - 'when execGit returns timedOut:true — not the generic git_list_failed', - ].join(' ') - ); - }); - - test('listLinkedWorktreePaths reason is git_timed_out when execGit returns timedOut:true', () => { - const { listLinkedWorktreePaths } = require(WORKTREE_SAFETY_PATH); - - const result = listLinkedWorktreePaths('/tmp', { execGit: makeTimeoutStub() }); - - assert.strictEqual( - result.reason, - 'git_timed_out', - [ - 'AC4 (strict): listLinkedWorktreePaths must use reason=git_timed_out', - 'when execGit returns timedOut:true', - ].join(' ') - ); - }); - - test('executeWorktreePrunePlan result.timedOut is true when prune git call times out', () => { - const { executeWorktreePrunePlan } = require(WORKTREE_SAFETY_PATH); - - // Use a plan that bypasses readWorktreeList (action=metadata_prune_only) - // so the prune execGit call itself can time out - const plan = { - repoRoot: '/tmp', - action: 'metadata_prune_only', - reason: 'no_worktrees', - destructiveModeRequested: false, - }; - - const result = executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() }); - - assert.strictEqual(result.ok, false, 'executeWorktreePrunePlan must return ok:false when prune times out'); - - // AC4 strict: timedOut must be surfaced as a first-class field - assert.strictEqual( - result.timedOut, - true, - [ - 'AC4 (strict): executeWorktreePrunePlan must include timedOut:true in result', - 'when the execGit call returns timedOut:true', - ].join(' ') - ); - }); - - test('snapshotWorktreeInventory reason is git_timed_out on timeout', () => { - const { snapshotWorktreeInventory } = require(WORKTREE_SAFETY_PATH); - - const result = snapshotWorktreeInventory('/tmp', {}, { execGit: makeTimeoutStub() }); - - assert.strictEqual( - result.reason, - 'git_timed_out', - [ - 'AC4 (strict): snapshotWorktreeInventory must use reason=git_timed_out', - 'when execGit returns timedOut:true', - ].join(' ') - ); - }); -}); - -// ─── AC3: non-crashing under degraded git — worktree prune flow ─────────────── - -describe('bug-3281 AC3: worktree prune flow is non-crashing under degraded git', () => { - test('full prune flow (plan -> execute) completes without throwing on timeout', () => { - const { planWorktreePrune, executeWorktreePrunePlan } = require(WORKTREE_SAFETY_PATH); - - let threw = false; - try { - const plan = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); - executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() }); - } catch { - threw = true; - } - - assert.strictEqual(threw, false, 'full prune flow must not throw on timeout — must degrade gracefully'); - }); - - test('inspectWorktreeHealth findings is empty array (not undefined) on timeout', () => { - const { inspectWorktreeHealth } = require(WORKTREE_SAFETY_PATH); - - const result = inspectWorktreeHealth('/tmp', {}, { execGit: makeTimeoutStub() }); - - // ok:false is expected — but findings must still be an array (not undefined) - // so callers that iterate findings do not crash - assert.strictEqual(Array.isArray(result.findings), true, 'findings must be an array even when ok:false'); - }); -}); diff --git a/tests/bug-3384-worktree-cleanup-manifest.test.cjs b/tests/bug-3384-worktree-cleanup-manifest.test.cjs deleted file mode 100644 index 51674adb3..000000000 --- a/tests/bug-3384-worktree-cleanup-manifest.test.cjs +++ /dev/null @@ -1,235 +0,0 @@ -// allow-test-rule: source-text-is-the-product -// Workflow markdown is the installed orchestration contract, and the CJS policy -// module is the callable safety seam for worktree cleanup. - -'use strict'; - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); - -const { - planWorktreeWaveCleanup, - executeWorktreeWaveCleanupPlan, -} = require('../get-shit-done/bin/lib/worktree-safety.cjs'); - -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'); - -function readWorkflow(filePath) { - return fs.readFileSync(filePath, 'utf8'); -} - -describe('bug #3384: worktree cleanup is manifest-scoped and fail-closed', () => { - test('cleanup plan includes only manifest entries and never discovers global agent worktrees', () => { - const plan = planWorktreeWaveCleanup('/repo/main', { - worktrees: [ - { - agent_id: 'a1', - worktree_path: '/repo/.claude/worktrees/agent-a1', - branch: 'worktree-agent-a1', - expected_base: 'abc123', - }, - ], - }); - - assert.equal(plan.ok, true); - assert.deepEqual(plan.entries.map((entry) => ({ - agent_id: entry.agent_id, - worktree_path: entry.worktree_path, - branch: entry.branch, - expected_base: entry.expected_base, - })), [{ - agent_id: 'a1', - worktree_path: '/repo/.claude/worktrees/agent-a1', - branch: 'worktree-agent-a1', - expected_base: 'abc123', - }]); - assert.equal(plan.discovery, 'manifest'); - }); - - test('cleanup plan rejects entries without expected base or disposable branch namespace', () => { - const plan = planWorktreeWaveCleanup('/repo/main', { - worktrees: [ - { - agent_id: 'missing-base', - worktree_path: '/repo/.claude/worktrees/agent-missing-base', - branch: 'worktree-agent-missing-base', - }, - { - agent_id: 'feature-branch', - worktree_path: '/repo/.claude/worktrees/agent-feature', - branch: 'feature/user-work', - expected_base: 'abc123', - }, - ], - }); - - assert.equal(plan.ok, false); - assert.equal(plan.reason, 'empty_manifest'); - assert.deepEqual(plan.entries, []); - }); - - test('cleanup executor does not delete a branch when worktree removal fails', () => { - const calls = []; - const plan = { - ok: true, - repoRoot: '/repo/main', - action: 'cleanup_wave', - discovery: 'manifest', - entries: [{ - agent_id: 'a1', - worktree_path: '/repo/.claude/worktrees/agent-a1', - branch: 'worktree-agent-a1', - expected_base: 'abc123', - }], - }; - - const result = executeWorktreeWaveCleanupPlan(plan, { - execGit: (args, opts) => { - calls.push({ cwd: opts.cwd, args }); - const key = args.join(' '); - if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { - return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; - } - if (key === 'merge-base HEAD worktree-agent-a1') { - return { exitCode: 0, stdout: 'abc123', stderr: '' }; - } - if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { - return { exitCode: 0, stdout: '', stderr: '' }; - } - if (key.startsWith('merge worktree-agent-a1')) { - return { exitCode: 0, stdout: '', stderr: '' }; - } - if (key === 'worktree remove /repo/.claude/worktrees/agent-a1 --force') { - return { exitCode: 1, stdout: '', stderr: 'locked' }; - } - if (key === 'branch -D worktree-agent-a1') { - throw new Error('branch deletion must not run after remove failure'); - } - return { exitCode: 0, stdout: '', stderr: '' }; - }, - }); - - assert.equal(result.ok, false); - assert.equal(result.entries[0].status, 'blocked'); - assert.equal(result.entries[0].reason, 'worktree_remove_failed'); - assert.equal(calls.some((call) => call.args.join(' ') === 'branch -D worktree-agent-a1'), false); - }); - - test('cleanup executor stops on merge conflict and records remaining manifest entries', () => { - const plan = { - ok: true, - repoRoot: '/repo/main', - action: 'cleanup_wave', - discovery: 'manifest', - entries: [ - { - agent_id: 'a1', - worktree_path: '/repo/.claude/worktrees/agent-a1', - branch: 'worktree-agent-a1', - expected_base: 'abc123', - }, - { - agent_id: 'a2', - worktree_path: '/repo/.claude/worktrees/agent-a2', - branch: 'worktree-agent-a2', - expected_base: 'abc123', - }, - ], - }; - - const result = executeWorktreeWaveCleanupPlan(plan, { - execGit: (args) => { - const key = args.join(' '); - if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { - return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; - } - if (key === 'merge-base HEAD worktree-agent-a1') { - return { exitCode: 0, stdout: 'abc123', stderr: '' }; - } - if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { - return { exitCode: 0, stdout: '', stderr: '' }; - } - if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') { - return { exitCode: 0, stdout: '', stderr: '' }; - } - if (key.startsWith('merge worktree-agent-a1')) { - return { exitCode: 1, stdout: '', stderr: 'CONFLICT' }; - } - throw new Error(`unexpected git call after conflict: ${key}`); - }, - }); - - assert.equal(result.ok, false); - assert.equal(result.entries[0].status, 'blocked'); - assert.equal(result.entries[0].reason, 'merge_failed'); - assert.deepEqual(result.pending.map((entry) => entry.branch), ['worktree-agent-a2']); - }); - - test('cleanup executor blocks dirty worktrees before merge/remove/delete', () => { - const calls = []; - const plan = { - ok: true, - repoRoot: '/repo/main', - action: 'cleanup_wave', - discovery: 'manifest', - entries: [{ - agent_id: 'a1', - worktree_path: '/repo/.claude/worktrees/agent-a1', - branch: 'worktree-agent-a1', - expected_base: 'abc123', - }], - }; - - const result = executeWorktreeWaveCleanupPlan(plan, { - execGit: (args) => { - calls.push(args.join(' ')); - const key = args.join(' '); - if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { - return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; - } - if (key === 'merge-base HEAD worktree-agent-a1') { - return { exitCode: 0, stdout: 'abc123', stderr: '' }; - } - if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { - return { exitCode: 0, stdout: '', stderr: '' }; - } - if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') { - return { exitCode: 0, stdout: '?? scratch.txt', stderr: '' }; - } - throw new Error(`unexpected git call after dirty check: ${key}`); - }, - }); - - assert.equal(result.ok, false); - assert.equal(result.entries[0].reason, 'worktree_dirty'); - assert.equal(calls.some((call) => call.startsWith('merge worktree-agent-a1')), false); - assert.equal(calls.some((call) => call === 'worktree remove /repo/.claude/worktrees/agent-a1 --force'), false); - assert.equal(calls.some((call) => call === 'branch -D worktree-agent-a1'), false); - }); - - test('execute-phase contract requires a cleanup manifest instead of global worktree discovery', () => { - const content = readWorkflow(EXECUTE_PHASE_PATH); - assert.match(content, /WAVE_WORKTREE_MANIFEST/); - assert.match(content, /worktree\.cleanup-wave/); - assert.match(content, /atomically append `\{agent_id, worktree_path, branch, expected_base\}`/); - assert.match(content, /try\{if\(!p\)throw new Error\("WAVE_WORKTREE_MANIFEST is unset"\)/); - assert.match(content, /WT_PATHS_FILE=.*gsd-worktree-paths-/); - assert.doesNotMatch(content, /done < <\(node -e 'const fs=require\("fs"\);const p=process\.env\.WAVE_WORKTREE_MANIFEST/); - assert.doesNotMatch(content, /done < <\(git worktree list --porcelain \| grep "\^worktree " \| grep "\\\.claude\/worktrees\/agent-"/); - }); - - test('quick contract requires a cleanup manifest instead of global worktree discovery', () => { - const content = readWorkflow(QUICK_PATH); - assert.match(content, /WAVE_WORKTREE_MANIFEST|QUICK_WORKTREE_MANIFEST/); - assert.match(content, /worktree\.cleanup-wave/); - assert.match(content, /mktemp "\$\{TMPDIR:-\/tmp\}\/gsd-quick-worktree-/); - assert.match(content, /append its returned `\{agent_id, worktree_path, branch, expected_base\}`/); - assert.match(content, /try\{if\(!p\)throw new Error\("QUICK_WORKTREE_MANIFEST is unset"\)/); - assert.match(content, /WT_PATHS_FILE=.*gsd-worktree-paths-/); - assert.doesNotMatch(content, /done < <\(node -e 'const fs=require\("fs"\);const p=process\.env\.QUICK_WORKTREE_MANIFEST/); - assert.doesNotMatch(content, /done < <\(git worktree list --porcelain \| grep "\^worktree " \| grep "\\\.claude\/worktrees\/agent-"/); - }); -}); diff --git a/tests/bug-3425-worktree-cleanup-cwd-pin.test.cjs b/tests/bug-3425-worktree-cleanup-cwd-pin.test.cjs deleted file mode 100644 index bc501a5a0..000000000 --- a/tests/bug-3425-worktree-cleanup-cwd-pin.test.cjs +++ /dev/null @@ -1,36 +0,0 @@ -// allow-test-rule: source-text-is-the-product -// execute-phase.md is the shipped orchestration contract; this regression test -// locks the cleanup guards that prevent merge-loop CWD drift from targeting -// the wrong worktree/branch. - -'use strict'; - -const { test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); - -const EXECUTE_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md'); - -function readWorkflow() { - return fs.readFileSync(EXECUTE_PHASE_PATH, 'utf8'); -} - -test('#3425: helper cleanup path pins orchestrator CWD to primary worktree and checks EXPECTED_BRANCH', () => { - const content = readWorkflow(); - - assert.match(content, /PRIMARY_WT=\$\(git worktree list --porcelain \| awk '\/\^worktree \/\{print substr\(\$0,10\); exit\}'\)/); - assert.match(content, /if \[ -z "\$PRIMARY_WT" \]; then\s+echo "FATAL: could not resolve primary worktree before cleanup" >&2\s+exit 1\s+fi/); - assert.match(content, /cd "\$PRIMARY_WT" \|\| \{ echo "FATAL: cannot cd to primary worktree \$PRIMARY_WT" >&2; exit 1; \}/); - assert.match(content, /ORCH_BRANCH=\$\(git rev-parse --abbrev-ref HEAD\)/); - assert.match(content, /FATAL: orchestrator on '\$ORCH_BRANCH' but expected '\$EXPECTED_BRANCH' before worktree cleanup — refusing to merge \(#3174-class drift\)/); - assert.match(content, /gsd-sdk query worktree\.cleanup-wave --manifest "\$WAVE_WORKTREE_MANIFEST" \|\| exit 1/); -}); - -test('#3425: cleanup-tail snippet carries the same primary-worktree pin before removal', () => { - const content = readWorkflow(); - - assert.match(content, /Cleanup-tail: pin orchestrator CWD to primary worktree before cleanup-tail \(#3174\)\./); - assert.match(content, /FATAL: cannot cd to primary worktree \$PRIMARY_WT/); - assert.match(content, /# Cleanup-tail: remove residual agent worktrees after a cross-wave-dependency deviation\./); -}); diff --git a/tests/worktree-cleanup.test.cjs b/tests/worktree-cleanup.test.cjs index 72d97f448..c4211d9ed 100644 --- a/tests/worktree-cleanup.test.cjs +++ b/tests/worktree-cleanup.test.cjs @@ -1,21 +1,461 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may -// reclassify some entries as source-text-is-the-product during migration. +// allow-test-rule: source-text-is-the-product +// Workflow markdown is the installed orchestration contract. + +'use strict'; /** - * GSD Tools Tests - worktree cleanup after executor completes + * Worktree Cleanup Module — HEAD attachment, post-executor cleanup, and contract tests * - * Validates that execute-phase.md and quick.md include post-execution - * worktree cleanup logic (merge branch, remove worktree, delete branch). + * Seam: get-shit-done/workflows/{execute-phase,execute-plan,quick}.md, + * agents/gsd-executor.md, references/git-integration.md * - * Closes: #1496 + * Split from the consolidated 13→2 worktree cluster (≤800 LOC/file): + * - tests/bug-2924-worktree-head-attachment.test.cjs (#2924: HEAD attachment) + * - tests/worktree-cleanup.test.cjs (#1496: post-executor cleanup) + * - tests/worktree-merge-protection.test.cjs (#1756: orchestrator file protection) + * - tests/worktree-safety.test.cjs (#1977: commit safety hardening) + * - tests/worktree-stagger.test.cjs (#1511: sequential dispatch) + * - tests/bug-3384-worktree-cleanup-manifest.test.cjs (workflow contract side) + * - tests/bug-3425-worktree-cleanup-cwd-pin.test.cjs (#3425: CWD pin) + * + * See also: worktree.test.cjs (#2015, #2075, #2431, #2774) + * worktree-safety.test.cjs (safety function unit tests) */ -const { test, describe } = require('node:test'); +const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); +const fs = require('node:fs'); +const path = require('node:path'); + +const REPO_ROOT = path.join(__dirname, '..'); +const EXECUTE_PHASE_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'execute-phase.md'); +const EXECUTE_PLAN_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'execute-plan.md'); +const QUICK_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'quick.md'); +const EXECUTOR_AGENT_PATH = path.join(REPO_ROOT, 'agents', 'gsd-executor.md'); +const GIT_INTEGRATION_PATH = path.join(REPO_ROOT, 'get-shit-done', 'references', 'git-integration.md'); + +// ─── Helpers ────────────────────────────────────────────────────────────────── + +function extractNamedBlock(markdown, blockName) { + const open = `<${blockName}>`; + const close = ``; + const start = markdown.indexOf(open); + if (start === -1) return null; + const end = markdown.indexOf(close, start + open.length); + if (end === -1) return null; + return markdown.slice(start + open.length, end); +} + +/** + * Extract all fenced code blocks (```...```) from a markdown chunk. + * Returns array of { lang, body } objects. + */ +function extractFencedCodeBlocks(markdown) { + const blocks = []; + const lines = markdown.split('\n'); + let inFence = false; + let fenceLang = ''; + let buffer = []; + for (const line of lines) { + const trimmed = line.trimStart(); + if (trimmed.startsWith('```')) { + if (!inFence) { + inFence = true; + fenceLang = trimmed.slice(3).trim(); + buffer = []; + } else { + blocks.push({ lang: fenceLang, body: buffer.join('\n') }); + inFence = false; + fenceLang = ''; + buffer = []; + } + } else if (inFence) { + buffer.push(line); + } + } + return blocks; +} + +/** + * Tokenize a shell-like script into individual statements (split on `;`, `&&`, `||`, newlines) + * and return commands as arrays of word tokens. + */ +function shellStatements(script) { + const statements = []; + const lines = script.split('\n'); + for (let raw of lines) { + const line = raw.replace(/#.*$/, '').trim(); + if (!line) continue; + const parts = line.split(/(?:&&|\|\||;)/); + for (const part of parts) { + let trimmed = part.trim(); + if (!trimmed) continue; + const assignMatch = trimmed.match(/^[A-Za-z_][A-Za-z0-9_]*=(.*)$/); + if (assignMatch) trimmed = assignMatch[1]; + const subMatch = trimmed.match(/^\$\((.*?)\)?$/); + if (subMatch) trimmed = subMatch[1]; + if (trimmed.startsWith('$(')) trimmed = trimmed.slice(2); + trimmed = trimmed.replace(/\)+\s*$/, '').trim(); + if (!trimmed) continue; + statements.push(trimmed.split(/\s+/).filter(Boolean)); + } + } + return statements; +} + +/** + * Find the line index of the first command matching a predicate. + * Returns -1 when not found. + */ +function findCommandIndex(statements, predicate) { + for (let i = 0; i < statements.length; i++) { + if (predicate(statements[i])) return i; + } + return -1; +} + +// ─── #2924: HEAD attachment + destructive recovery ────────────────────────── + +describe('bug #2924: worktree HEAD attachment + destructive recovery', () => { + describe('execute-phase.md worktree_branch_check', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const block = extractNamedBlock(content, 'worktree_branch_check'); + + test('block exists', () => { + assert.ok(block, 'execute-phase.md must contain a block'); + }); + + test('block invokes `git symbolic-ref` to inspect HEAD attachment', () => { + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const idx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') + ); + assert.notStrictEqual( + idx, -1, + 'worktree_branch_check must run `git symbolic-ref ... HEAD` to verify HEAD attachment before any reset' + ); + }); + + test('HEAD-attachment assertion runs BEFORE `git reset --hard`', () => { + const codeBlocks = extractFencedCodeBlocks(block); + const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body)); + const symbolicRefIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') + ); + const resetHardIdx = findCommandIndex(allStatements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'reset' && cmd.includes('--hard') + ); + assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref check must exist'); + assert.notStrictEqual(resetHardIdx, -1, 'reset --hard must exist'); + assert.ok( + symbolicRefIdx < resetHardIdx, + 'HEAD attachment assertion (symbolic-ref) must precede `git reset --hard` so a stale HEAD never moves a protected branch' + ); + }); + + test('block names protected branches that must NOT be the agent branch', () => { + // The protected-branch list must be enforced by name. Parse it out of the + // shell scripts and verify required names are present. + const codeBlocks = extractFencedCodeBlocks(block); + const scripts = codeBlocks.map(({ body }) => body).join('\n'); + // Look for an assignment whose value is a regex/list naming protected refs. + // Acceptable forms: PROTECTED_BRANCHES_RE='...' or grep -Eq '^(main|...)$' + // Parse the alternation list out of the grep -E pattern so we assert + // structurally on the protected-branch enumeration rather than via + // raw substring matching (release/* contains regex-special chars and + // can't be safely tested with `\b...\b`). + const altMatch = scripts.match(/grep\s+-Eq?\s+'\^\(([^)]+)\)\$'/); + assert.ok( + altMatch, + 'worktree_branch_check must contain a `grep -Eq` protected-branch alternation pattern' + ); + const branches = altMatch[1].split('|').map((b) => b.trim()); + const required = ['main', 'master', 'develop', 'trunk', 'release/.*']; + for (const name of required) { + assert.ok( + branches.includes(name), + `worktree_branch_check protected-branch alternation must include '${name}' (found: ${branches.join(', ')})` + ); + } + }); + + test('block enforces positive worktree-agent-* allow-list (#2924 hardening)', () => { + const codeBlocks = extractFencedCodeBlocks(block); + const scripts = codeBlocks.map(({ body }) => body).join('\n'); + // Allow-list must reference the canonical Claude Code worktree-agent- + // namespace via a regex assertion (grep -Eq '^worktree-agent-...'). + const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; + assert.ok( + allowListRe.test(scripts), + 'worktree_branch_check must enforce a positive allow-list matching ^worktree-agent-* (#2924 hardening)' + ); + }); + + test('block forbids `git update-ref` self-recovery in its guidance text', () => { + // The forbidding statement is documentation text, not a shell command, + // so structural shell parsing does not apply. Verify the prohibition + // appears as standalone guidance somewhere in the block. + assert.ok( + block.includes('update-ref'), + 'worktree_branch_check must explicitly forbid `git update-ref` self-recovery' + ); + }); + }); + + describe('execute-phase.md no longer defaults to --no-verify in parallel mode', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const block = extractNamedBlock(content, 'parallel_execution'); + + test('parallel_execution block exists', () => { + assert.ok(block, 'execute-phase.md must contain a block'); + }); + + test('parallel_execution does NOT instruct agents to use --no-verify by default', () => { + // Tokenize the block as plain words and look for an unconditional + // imperative naming `--no-verify`. The acceptable presence is in a + // negated/opt-out context (e.g. "Do NOT pass --no-verify"); reject + // any sentence whose first verb is "Use --no-verify". + const sentences = block + .replace(/\n+/g, ' ') + .split(/(?<=[.!?])\s+/); + for (const sentence of sentences) { + if (!sentence.includes('--no-verify')) continue; + const lower = sentence.toLowerCase(); + const isProhibition = + /\b(do not|don't|never|no longer)\b/.test(lower) || + /\bopt[\s-]?out\b/.test(lower) || + /\bopt[\s-]?in\b/.test(lower) || + /\bif\b/.test(lower); + assert.ok( + isProhibition, + `parallel_execution sentence appears to mandate --no-verify by default: "${sentence.trim()}"` + ); + } + }); + }); + + describe('execute-plan.md no longer mandates --no-verify for parallel executor', () => { + const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); + const block = extractNamedBlock(content, 'precommit_failure_handling'); + test('precommit_failure_handling block exists', () => { + assert.ok(block, 'execute-plan.md must contain a block'); + }); + + test('parallel-executor sub-section does not unconditionally mandate --no-verify', () => { + // Locate the parallel-executor sub-section heading and parse the + // sentences under it. + const headingIdx = block.indexOf('parallel executor'); + assert.notStrictEqual(headingIdx, -1, 'must contain a parallel-executor sub-section'); + const endIdx = block.indexOf('**If running as the sole', headingIdx); + assert.notStrictEqual(endIdx, -1, 'parallel-executor sub-section terminator must exist'); + const subBlock = block.slice(headingIdx, endIdx); + assert.ok(subBlock.length > 0, 'sub-section must have content'); + const sentences = subBlock.replace(/\n+/g, ' ').split(/(?<=[.!?])\s+/); + for (const sentence of sentences) { + if (!sentence.includes('--no-verify')) continue; + const lower = sentence.toLowerCase(); + const isProhibition = + /\b(do not|don't|never|no longer)\b/.test(lower) || + /\bopt[\s-]?out\b/.test(lower) || + /\bopt[\s-]?in\b/.test(lower) || + /\bif\b/.test(lower); + assert.ok( + isProhibition, + `parallel-executor guidance sentence appears to mandate --no-verify: "${sentence.trim()}"` + ); + } + }); + }); + + describe('quick.md worktree_branch_check', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const block = extractNamedBlock(content, 'worktree_branch_check'); + + test('block exists', () => { + assert.ok(block, 'quick.md must contain a block'); + }); + + test('block references `git symbolic-ref` for HEAD attachment assertion', () => { + // quick.md uses inline `git symbolic-ref ... HEAD` rather than a fenced + // block, so search the block as a token stream of statements. + const statements = shellStatements(block); + const idx = findCommandIndex(statements, (cmd) => + cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD') + ); + assert.notStrictEqual( + idx, -1, + 'quick.md worktree_branch_check must run `git symbolic-ref ... HEAD`' + ); + }); + + test('HEAD assertion precedes `git reset --hard`', () => { + const symbolicRefByteIdx = block.indexOf('symbolic-ref'); + const resetHardByteIdx = block.indexOf('reset --hard'); + assert.notStrictEqual(symbolicRefByteIdx, -1); + assert.notStrictEqual(resetHardByteIdx, -1); + assert.ok( + symbolicRefByteIdx < resetHardByteIdx, + 'symbolic-ref HEAD assertion must appear before `git reset --hard` in quick.md worktree_branch_check' + ); + }); + + test('block forbids `git update-ref` self-recovery', () => { + assert.ok( + block.includes('update-ref'), + 'quick.md worktree_branch_check must explicitly forbid `git update-ref` self-recovery' + ); + }); + + test('block enforces positive worktree-agent-* allow-list (#2924 hardening)', () => { + const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; + assert.ok( + allowListRe.test(block), + 'quick.md worktree_branch_check must enforce a positive allow-list matching ^worktree-agent-* (#2924 hardening)' + ); + }); + }); + + describe('quick.md pre-dispatch plan commit no longer hard-codes --no-verify', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const codeBlocks = extractFencedCodeBlocks(content); + // Find the bash block containing the pre-dispatch plan commit + const target = codeBlocks.find(({ body }) => + body.includes('pre-dispatch plan') && body.includes('git commit') + ); + test('pre-dispatch plan commit block exists', () => { + assert.ok(target, 'quick.md must contain the pre-dispatch plan commit block'); + }); + + test('pre-dispatch plan commit gates --no-verify behind a config flag', () => { + // The block must contain BOTH a `git commit` without --no-verify AND + // gate any --no-verify variant inside an `if` block reading a config + // value (workflow.worktree_skip_hooks). + const statements = shellStatements(target.body); + const noVerifyCommits = statements.filter((cmd) => + cmd[0] === 'git' && cmd[1] === 'commit' && cmd.includes('--no-verify') + ); + const cleanCommits = statements.filter((cmd) => + cmd[0] === 'git' && cmd[1] === 'commit' && !cmd.includes('--no-verify') + ); + assert.ok( + cleanCommits.length >= 1, + 'must include at least one `git commit` without --no-verify (default path)' + ); + // If --no-verify still appears, the block must reference the opt-in flag. + if (noVerifyCommits.length > 0) { + assert.ok( + target.body.includes('worktree_skip_hooks'), + '--no-verify commits must be gated behind workflow.worktree_skip_hooks config flag' + ); + } + }); + }); + + describe('gsd-executor.md prohibits update-ref self-recovery', () => { + const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); + const block = extractNamedBlock(content, 'destructive_git_prohibition'); + + test('destructive_git_prohibition block exists', () => { + assert.ok(block, 'gsd-executor.md must contain a block'); + }); + + test('block prohibits `git update-ref refs/heads/`', () => { + assert.ok( + block.includes('update-ref'), + 'destructive_git_prohibition must enumerate `git update-ref` as a prohibited command' + ); + assert.ok( + block.includes('protected') || block.includes('main') || block.includes('master'), + 'destructive_git_prohibition must call out protected branches in the update-ref prohibition' + ); + }); + + test('block references issue #2924', () => { + assert.ok( + block.includes('#2924'), + 'destructive_git_prohibition should cite #2924 as the source of the update-ref prohibition' + ); + }); + }); + + describe('gsd-executor.md task_commit_protocol enforces worktree-agent-* allow-list', () => { + const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); + const block = extractNamedBlock(content, 'task_commit_protocol'); + + test('task_commit_protocol block exists', () => { + assert.ok(block, 'gsd-executor.md must contain a block'); + }); + + test('step 0 enforces positive worktree-agent-* allow-list (#2924 hardening)', () => { + const codeBlocks = extractFencedCodeBlocks(block); + const scripts = codeBlocks.map(({ body }) => body).join('\n'); + const allowListRe = /grep\s+-Eq?\s+'\^worktree-agent-/; + assert.ok( + allowListRe.test(scripts), + 'task_commit_protocol step 0 must enforce a positive allow-list matching ^worktree-agent-* in addition to the protected-ref deny-list (#2924 hardening)' + ); + }); + }); + + describe('no workflow file performs unconditional update-ref on a protected branch', () => { + const workflowsDir = path.join(REPO_ROOT, 'get-shit-done', 'workflows'); + const workflowFiles = fs + .readdirSync(workflowsDir, { recursive: true }) + .filter((f) => typeof f === 'string' && f.endsWith('.md')) + .map((f) => path.join(workflowsDir, f)); + + for (const filePath of workflowFiles) { + test(`${path.basename(filePath)} contains no update-ref of a protected ref`, () => { + const content = fs.readFileSync(filePath, 'utf-8'); + const blocks = extractFencedCodeBlocks(content); + for (const { body } of blocks) { + const statements = shellStatements(body); + for (const cmd of statements) { + if (cmd[0] !== 'git') continue; + if (cmd[1] !== 'update-ref') continue; + // Reject any update-ref that targets a protected ref. + const target = cmd[2] || ''; + const protectedRe = /^refs\/heads\/(main|master|develop|trunk|release\/.+)$/; + assert.ok( + !protectedRe.test(target), + `${path.basename(filePath)} contains forbidden 'git update-ref ${target}' (#2924)` + ); + } + } + }); + } + }); + + describe('git-integration.md guidance reflects new default', () => { + const content = fs.readFileSync(GIT_INTEGRATION_PATH, 'utf-8'); + test('parallel-agents guidance no longer mandates --no-verify', () => { + // Find the parallel-agents callout and parse its sentences. + const idx = content.indexOf('Parallel agents'); + assert.notStrictEqual(idx, -1, 'must contain a "Parallel agents" callout'); + const section = content.slice(idx); + const endMatch = section.slice(1).match(/\n#{1,6}\s/); + assert.ok(endMatch, 'Parallel agents section must terminate at the next heading'); + const tail = section.slice(0, 1 + endMatch.index); + const sentences = tail.replace(/\n+/g, ' ').split(/(?<=[.!?])\s+/); + for (const sentence of sentences) { + if (!sentence.includes('--no-verify')) continue; + const lower = sentence.toLowerCase(); + const isProhibition = + /\b(do not|don't|never|no longer)\b/.test(lower) || + /\bopt[\s-]?out\b/.test(lower) || + /\bopt[\s-]?in\b/.test(lower) || + /\bif\b/.test(lower); + assert.ok( + isProhibition, + `git-integration.md "Parallel agents" sentence appears to mandate --no-verify: "${sentence.trim()}"` + ); + } + }); + }); +}); + +// ─── #1496: post-executor worktree cleanup ────────────────────────────────── describe('worktree cleanup after executor completes (#1496)', () => { const executePhasePath = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md'); @@ -73,3 +513,212 @@ describe('worktree cleanup after executor completes (#1496)', () => { 'cleanup should discover worktrees via git worktree list'); }); }); + +// ─── #1756: orchestrator file protection during merge ──────────────────────── + +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'); + }); +}); + +// ─── #1977: commit safety hardening ───────────────────────────────────────── + +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'); + assert.ok(content.includes('worktree_branch_check'), 'execute-plan.md must contain a worktree_branch_check block'); + 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'); + 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'); + assert.ok(content.includes('--diff-filter=D'), 'must include --diff-filter=D deletion verification'); + assert.ok( + content.includes('WARNING') || content.includes('DELETIONS'), + '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'); + const mergeIdx = content.indexOf('git merge'); + assert.ok(mergeIdx > -1, 'must contain a git merge operation'); + const worktreeCleanupStart = content.indexOf('Worktree cleanup'); + assert.ok(worktreeCleanupStart > -1, 'must have a worktree cleanup section'); + const cleanupSection = content.slice(worktreeCleanupStart); + assert.ok(cleanupSection.includes('--diff-filter=D'), 'must include --diff-filter=D to check for deletions before merge'); + const deletionCheckIdx = cleanupSection.indexOf('--diff-filter=D'); + const gitMergeIdx = cleanupSection.indexOf('git merge'); + assert.ok(deletionCheckIdx < gitMergeIdx, 'deletion check must appear before git merge'); + assert.ok( + cleanupSection.includes('BLOCKED') || cleanupSection.includes('DELETIONS') || cleanupSection.includes('deletion'), + 'must warn or block when the worktree branch contains file deletions' + ); + }); +}); + + +// ─── #1511: sequential dispatch ───────────────────────────────────────────── + +describe('worktree sequential dispatch', () => { + test('execute-phase.md exists', () => { + assert.ok(fs.existsSync(EXECUTE_PHASE_PATH), 'execute-phase.md should exist'); + }); + + test('execute-phase explains git config.lock contention', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok(content.includes('config.lock'), 'should explain the git config.lock race condition'); + }); + + test('execute-phase requires sequential dispatch with run_in_background', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok(content.includes('run_in_background'), 'should instruct one-at-a-time dispatch with run_in_background'); + }); + + test('execute-phase warns against multiple Task calls in single message', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok( + content.includes('WRONG') && content.includes('single message'), + 'should warn against sending multiple Task() calls simultaneously' + ); + }); +}); + + +// ─── #3384: cleanup manifest workflow contracts ────────────────────────────── + +describe('bug #3384: worktree cleanup workflow contracts', () => { + test('execute-phase contract requires a cleanup manifest instead of global worktree discovery', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf8'); + assert.match(content, /WAVE_WORKTREE_MANIFEST/); + assert.match(content, /worktree\.cleanup-wave/); + assert.match(content, /atomically append `\{agent_id, worktree_path, branch, expected_base\}`/); + assert.match(content, /try\{if\(!p\)throw new Error\("WAVE_WORKTREE_MANIFEST is unset"\)/); + assert.match(content, /WT_PATHS_FILE=.*gsd-worktree-paths-/); + assert.doesNotMatch(content, /done < <\(node -e 'const fs=require\("fs"\);const p=process\.env\.WAVE_WORKTREE_MANIFEST/); + assert.doesNotMatch(content, /done < <\(git worktree list --porcelain \| grep "\^worktree " \| grep "\\\.claude\/worktrees\/agent-"/); + }); + + test('quick contract requires a cleanup manifest instead of global worktree discovery', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf8'); + assert.match(content, /WAVE_WORKTREE_MANIFEST|QUICK_WORKTREE_MANIFEST/); + assert.match(content, /worktree\.cleanup-wave/); + assert.match(content, /mktemp "\$\{TMPDIR:-\/tmp\}\/gsd-quick-worktree-/); + assert.match(content, /append its returned `\{agent_id, worktree_path, branch, expected_base\}`/); + assert.match(content, /try\{if\(!p\)throw new Error\("QUICK_WORKTREE_MANIFEST is unset"\)/); + assert.match(content, /WT_PATHS_FILE=.*gsd-worktree-paths-/); + assert.doesNotMatch(content, /done < <\(node -e 'const fs=require\("fs"\);const p=process\.env\.QUICK_WORKTREE_MANIFEST/); + assert.doesNotMatch(content, /done < <\(git worktree list --porcelain \| grep "\^worktree " \| grep "\\\.claude\/worktrees\/agent-"/); + }); +}); + + +// ─── #3425: CWD pin before cleanup ────────────────────────────────────────── + +test('#3425: helper cleanup path pins orchestrator CWD to primary worktree and checks EXPECTED_BRANCH', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf8'); + + assert.match(content, /PRIMARY_WT=\$\(git worktree list --porcelain \| awk '\/\^worktree \/\{print substr\(\$0,10\); exit\}'\)/); + assert.match(content, /if \[ -z "\$PRIMARY_WT" \]; then\s+echo "FATAL: could not resolve primary worktree before cleanup" >&2\s+exit 1\s+fi/); + assert.match(content, /cd "\$PRIMARY_WT" \|\| \{ echo "FATAL: cannot cd to primary worktree \$PRIMARY_WT" >&2; exit 1; \}/); + assert.match(content, /ORCH_BRANCH=\$\(git rev-parse --abbrev-ref HEAD\)/); + assert.match(content, /FATAL: orchestrator on '\$ORCH_BRANCH' but expected '\$EXPECTED_BRANCH' before worktree cleanup — refusing to merge \(#3174-class drift\)/); + assert.match(content, /gsd-sdk query worktree\.cleanup-wave --manifest "\$WAVE_WORKTREE_MANIFEST" \|\| exit 1/); +}); + +test('#3425: cleanup-tail snippet carries the same primary-worktree pin before removal', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf8'); + + assert.match(content, /Cleanup-tail: pin orchestrator CWD to primary worktree before cleanup-tail \(#3174\)\./); + assert.match(content, /FATAL: cannot cd to primary worktree \$PRIMARY_WT/); + assert.match(content, /# Cleanup-tail: remove residual agent worktrees after a cross-wave-dependency deviation\./); +}); diff --git a/tests/worktree-merge-protection.test.cjs b/tests/worktree-merge-protection.test.cjs deleted file mode 100644 index cd40420fc..000000000 --- a/tests/worktree-merge-protection.test.cjs +++ /dev/null @@ -1,106 +0,0 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may -// reclassify some entries as source-text-is-the-product during migration. - -/** - * 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'); - }); -}); diff --git a/tests/worktree-safety-policy.test.cjs b/tests/worktree-safety-policy.test.cjs deleted file mode 100644 index 6507b74f4..000000000 --- a/tests/worktree-safety-policy.test.cjs +++ /dev/null @@ -1,276 +0,0 @@ -'use strict'; - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); - -const { - resolveWorktreeContext, - parseWorktreePorcelain, - planWorktreePrune, - executeWorktreePrunePlan, - listLinkedWorktreePaths, - inspectWorktreeHealth, - snapshotWorktreeInventory, -} = require('../get-shit-done/bin/lib/worktree-safety.cjs'); - -const isWindows = process.platform === 'win32'; - -describe('worktree-safety policy module', () => { - test('resolveWorktreeContext prefers current directory when .planning exists', () => { - const context = resolveWorktreeContext('/repo/wt', { - existsSync: () => true, - execGit: () => ({ exitCode: 1, stdout: '', stderr: '' }), - }); - assert.strictEqual(context.effectiveRoot, '/repo/wt'); - assert.strictEqual(context.reason, 'has_local_planning'); - assert.strictEqual(context.mode, 'current_directory'); - }); - - test('resolveWorktreeContext maps linked worktree to common-dir parent', - { skip: isWindows ? 'POSIX-rooted fixture paths cannot be expressed on Windows path.resolve; resolveWorktreeContext uses platform-native path module and would prepend a drive letter to "/repo" inputs. Behaviour is covered indirectly by real-fs worktree tests.' : false }, - () => { - const context = resolveWorktreeContext('/repo/wt', { - existsSync: () => false, - execGit: (args) => { - if (args[1] === '--git-dir') return { exitCode: 0, stdout: '.git/worktrees/wt', stderr: '' }; - if (args[1] === '--git-common-dir') return { exitCode: 0, stdout: '../.git', stderr: '' }; - return { exitCode: 1, stdout: '', stderr: '' }; - }, - }); - assert.strictEqual(context.effectiveRoot, '/repo'); - assert.strictEqual(context.reason, 'linked_worktree'); - assert.strictEqual(context.mode, 'linked_worktree_root'); - }); - - test('resolveWorktreeContext falls back when git metadata is unavailable', () => { - const context = resolveWorktreeContext('/repo/wt', { - existsSync: () => false, - execGit: () => ({ exitCode: 1, stdout: '', stderr: '' }), - }); - assert.strictEqual(context.effectiveRoot, '/repo/wt'); - assert.strictEqual(context.reason, 'not_git_repo'); - }); - - test('resolveWorktreeContext keeps cwd for main worktree checkout', () => { - const context = resolveWorktreeContext('/repo/main', { - existsSync: () => false, - execGit: (args) => { - if (args[1] === '--git-dir') return { exitCode: 0, stdout: '.git', stderr: '' }; - if (args[1] === '--git-common-dir') return { exitCode: 0, stdout: '.git', stderr: '' }; - return { exitCode: 1, stdout: '', stderr: '' }; - }, - }); - assert.strictEqual(context.effectiveRoot, '/repo/main'); - assert.strictEqual(context.reason, 'main_worktree'); - assert.strictEqual(context.mode, 'current_directory'); - }); - - test('parseWorktreePorcelain skips detached HEAD entries', () => { - const porcelain = [ - 'worktree /repo/main', - 'HEAD deadbeef', - 'branch refs/heads/main', - '', - 'worktree /repo/wt-detached', - 'HEAD cafe1234', - 'detached', - '', - 'worktree /repo/wt-feature', - 'HEAD f00dbabe', - 'branch refs/heads/feature-x', - '', - ].join('\n'); - const parsed = parseWorktreePorcelain(porcelain); - assert.deepStrictEqual(parsed, [ - { path: '/repo/main', branch: 'main' }, - { path: '/repo/wt-feature', branch: 'feature-x' }, - ]); - }); - - test('planWorktreePrune is non-destructive by default', () => { - const plan = planWorktreePrune('/repo/main', {}, { - execGit: () => ({ exitCode: 0, stdout: 'worktree /repo/main\nbranch refs/heads/main\n', stderr: '' }), - parseWorktreePorcelain: () => [{ path: '/repo/main', branch: 'main' }], - }); - assert.strictEqual(plan.action, 'metadata_prune_only'); - assert.strictEqual(plan.reason, 'worktrees_present'); - assert.strictEqual(plan.destructiveModeRequested, false); - }); - - test('planWorktreePrune keeps metadata-prune action when destructive mode is requested (scaffold)', () => { - const plan = planWorktreePrune('/repo/main', { allowDestructive: true }, { - execGit: () => ({ exitCode: 0, stdout: '', stderr: '' }), - parseWorktreePorcelain: () => [], - }); - assert.strictEqual(plan.action, 'metadata_prune_only'); - assert.strictEqual(plan.reason, 'no_worktrees'); - assert.strictEqual(plan.destructiveModeRequested, true); - }); - - test('planWorktreePrune skips when git worktree list fails', () => { - const plan = planWorktreePrune('/repo/main', {}, { - execGit: () => ({ exitCode: 2, stdout: '', stderr: 'fatal' }), - }); - assert.strictEqual(plan.action, 'skip'); - assert.strictEqual(plan.reason, 'git_list_failed'); - }); - - test('planWorktreePrune still metadata-prunes when porcelain parser throws', () => { - const plan = planWorktreePrune('/repo/main', {}, { - execGit: () => ({ exitCode: 0, stdout: 'not-porcelain', stderr: '' }), - parseWorktreePorcelain: () => { - throw new Error('parse failed'); - }, - }); - assert.strictEqual(plan.action, 'metadata_prune_only'); - assert.strictEqual(plan.reason, 'no_worktrees'); - }); - - test('executeWorktreePrunePlan runs git worktree prune for metadata plan', () => { - const calls = []; - const result = executeWorktreePrunePlan( - { repoRoot: '/repo/main', action: 'metadata_prune_only', reason: 'worktrees_present' }, - { - execGit: (args, opts) => { - calls.push({ cwd: opts.cwd, args }); - return { exitCode: 0, stdout: '', stderr: '' }; - }, - } - ); - assert.strictEqual(result.ok, true); - assert.deepStrictEqual(calls, [{ cwd: '/repo/main', args: ['worktree', 'prune'] }]); - }); - - test('executeWorktreePrunePlan returns skip for missing plan', () => { - const result = executeWorktreePrunePlan(null, { - execGit: () => ({ exitCode: 0, stdout: '', stderr: '' }), - }); - assert.strictEqual(result.ok, false); - assert.strictEqual(result.action, 'skip'); - assert.strictEqual(result.reason, 'missing_plan'); - }); - - test('executeWorktreePrunePlan returns skip plan unchanged without git call', () => { - let called = false; - const result = executeWorktreePrunePlan( - { repoRoot: '/repo/main', action: 'skip', reason: 'git_list_failed' }, - { - execGit: () => { - called = true; - return { exitCode: 0, stdout: '', stderr: '' }; - }, - } - ); - assert.strictEqual(result.ok, false); - assert.strictEqual(result.action, 'skip'); - assert.strictEqual(result.reason, 'git_list_failed'); - assert.strictEqual(called, false); - }); - - test('executeWorktreePrunePlan rejects unsupported actions', () => { - const result = executeWorktreePrunePlan( - { repoRoot: '/repo/main', action: 'remove_missing_paths', reason: 'explicit' }, - { - execGit: () => ({ exitCode: 0, stdout: '', stderr: '' }), - } - ); - assert.strictEqual(result.ok, false); - assert.strictEqual(result.action, 'remove_missing_paths'); - assert.strictEqual(result.reason, 'unsupported_action'); - }); - - test('listLinkedWorktreePaths parses porcelain and skips first/main path', () => { - const listed = listLinkedWorktreePaths('/repo/main', { - execGit: () => ({ - exitCode: 0, - stdout: [ - 'worktree /repo/main', - 'HEAD aaa', - 'branch refs/heads/main', - '', - 'worktree /repo/wt-a', - 'HEAD bbb', - 'branch refs/heads/feat-a', - '', - 'worktree /repo/wt-b', - 'HEAD ccc', - 'detached', - '', - ].join('\n'), - stderr: '', - }), - }); - assert.strictEqual(listed.ok, true); - assert.deepStrictEqual(listed.paths, ['/repo/wt-a', '/repo/wt-b']); - }); - - test('inspectWorktreeHealth reports orphan and stale findings', () => { - const health = inspectWorktreeHealth( - '/repo/main', - { staleAfterMs: 60 * 60 * 1000, nowMs: 2 * 60 * 60 * 1000 }, - { - execGit: () => ({ - exitCode: 0, - stdout: [ - 'worktree /repo/main', - 'HEAD aaa', - 'branch refs/heads/main', - '', - 'worktree /repo/wt-orphan', - 'HEAD bbb', - 'branch refs/heads/feat-a', - '', - 'worktree /repo/wt-stale', - 'HEAD ccc', - 'branch refs/heads/feat-b', - '', - ].join('\n'), - stderr: '', - }), - existsSync: p => p !== '/repo/wt-orphan', - statSync: () => ({ mtimeMs: 0 }), - } - ); - - assert.strictEqual(health.ok, true); - assert.deepStrictEqual(health.findings, [ - { kind: 'orphan', path: '/repo/wt-orphan' }, - { kind: 'stale', path: '/repo/wt-stale', ageMinutes: 120 }, - ]); - }); - - test('snapshotWorktreeInventory returns typed linked-worktree entries', () => { - const inventory = snapshotWorktreeInventory( - '/repo/main', - { staleAfterMs: 60 * 60 * 1000, nowMs: 2 * 60 * 60 * 1000 }, - { - execGit: () => ({ - exitCode: 0, - stdout: [ - 'worktree /repo/main', - 'HEAD aaa', - 'branch refs/heads/main', - '', - 'worktree /repo/wt-a', - 'HEAD bbb', - 'branch refs/heads/feat-a', - '', - 'worktree /repo/wt-b', - 'HEAD ccc', - 'branch refs/heads/feat-b', - '', - ].join('\n'), - stderr: '', - }), - existsSync: p => p !== '/repo/wt-b', - statSync: () => ({ mtimeMs: 0 }), - } - ); - - assert.strictEqual(inventory.ok, true); - assert.deepStrictEqual(inventory.entries, [ - { path: '/repo/wt-a', exists: true, isStale: true, ageMinutes: 120 }, - { path: '/repo/wt-b', exists: false, isStale: false, ageMinutes: null }, - ]); - }); -}); diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index 0e272a69c..da53fc647 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -1,117 +1,693 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +'use strict'; /** - * Worktree commit safety hardening tests (#1977) + * Worktree Safety Policy Module — typed IR tests * - * 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) + * Seam: get-shit-done/bin/lib/worktree-safety.cjs + * Interface: resolveWorktreeContext, parseWorktreePorcelain, planWorktreePrune, + * executeWorktreePrunePlan, listLinkedWorktreePaths, inspectWorktreeHealth, + * snapshotWorktreeInventory, planWorktreeWaveCleanup, + * executeWorktreeWaveCleanupPlan + * + * Consolidated from: + * - tests/worktree-safety-policy.test.cjs (policy module unit tests) + * - tests/bug-3281-worktree-git-timeout.test.cjs (AC1–AC4: timeout/degraded-git) + * - tests/bug-3384-worktree-cleanup-manifest.test.cjs (manifest-scoped cleanup module) */ -'use strict'; - const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); +const path = require('node: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'); +const WORKTREE_SAFETY_PATH = path.join( + __dirname, '..', 'get-shit-done', 'bin', 'lib', 'worktree-safety.cjs' +); -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'); +const { + resolveWorktreeContext, + parseWorktreePorcelain, + planWorktreePrune, + executeWorktreePrunePlan, + listLinkedWorktreePaths, + inspectWorktreeHealth, + snapshotWorktreeInventory, + planWorktreeWaveCleanup, + executeWorktreeWaveCleanupPlan, +} = require(WORKTREE_SAFETY_PATH); - // The worktree_branch_check block must exist - assert.ok( - content.includes('worktree_branch_check'), - 'execute-plan.md must contain a worktree_branch_check block' - ); +const isWindows = process.platform === 'win32'; - // 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' - ); +// ─── Shared stubs ───────────────────────────────────────────────────────────── - // 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' - ); +/** + * Returns an execGit stub that simulates what spawnSync returns when the + * subprocess is killed by SIGTERM after exceeding its timeout. + * Per Node.js docs: result.status === null, result.signal === 'SIGTERM', + * result.error?.code === 'ETIMEDOUT'. + * + * The production execGit implementation must detect this shape and: + * - return { ..., timedOut: true } so callers can distinguish timeout from auth failure + * - not throw + */ +function makeTimeoutStub() { + return function stubTimedOutExecGit(_args, _opts) { + return { + exitCode: null, + stdout: '', + stderr: '', + timedOut: true, + signal: 'SIGTERM', + error: Object.assign(new Error('spawnSync git ETIMEDOUT'), { code: 'ETIMEDOUT' }), + }; + }; +} + +// ─── resolveWorktreeContext ─────────────────────────────────────────────────── + +describe('resolveWorktreeContext', () => { + test('prefers current directory when .planning exists', () => { + const context = resolveWorktreeContext('/repo/wt', { + existsSync: () => true, + execGit: () => ({ exitCode: 1, stdout: '', stderr: '' }), + }); + assert.strictEqual(context.effectiveRoot, '/repo/wt'); + assert.strictEqual(context.reason, 'has_local_planning'); + assert.strictEqual(context.mode, 'current_directory'); }); - 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('maps linked worktree to common-dir parent', + { skip: isWindows ? 'POSIX-rooted fixture paths cannot be expressed on Windows path.resolve' : false }, + () => { + const context = resolveWorktreeContext('/repo/wt', { + existsSync: () => false, + execGit: (args) => { + if (args[1] === '--git-dir') return { exitCode: 0, stdout: '.git/worktrees/wt', stderr: '' }; + if (args[1] === '--git-common-dir') return { exitCode: 0, stdout: '../.git', stderr: '' }; + return { exitCode: 1, stdout: '', stderr: '' }; + }, + }); + assert.strictEqual(context.effectiveRoot, '/repo'); + assert.strictEqual(context.reason, 'linked_worktree'); + assert.strictEqual(context.mode, 'linked_worktree_root'); }); - test('execute-phase.md worktree merge section includes pre-merge deletion check', () => { - const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + test('falls back when git metadata is unavailable', () => { + const context = resolveWorktreeContext('/repo/wt', { + existsSync: () => false, + execGit: () => ({ exitCode: 1, stdout: '', stderr: '' }), + }); + assert.strictEqual(context.effectiveRoot, '/repo/wt'); + assert.strictEqual(context.reason, 'not_git_repo'); + }); - // The merge section must exist - const mergeIdx = content.indexOf('git merge'); - assert.ok(mergeIdx > -1, 'execute-phase.md must contain a git merge operation'); + test('keeps cwd for main worktree checkout', () => { + const context = resolveWorktreeContext('/repo/main', { + existsSync: () => false, + execGit: (args) => { + if (args[1] === '--git-dir') return { exitCode: 0, stdout: '.git', stderr: '' }; + if (args[1] === '--git-common-dir') return { exitCode: 0, stdout: '.git', stderr: '' }; + return { exitCode: 1, stdout: '', stderr: '' }; + }, + }); + assert.strictEqual(context.effectiveRoot, '/repo/main'); + assert.strictEqual(context.reason, 'main_worktree'); + assert.strictEqual(context.mode, 'current_directory'); + }); - // 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'); + // Counter-test: timeout returns object with effectiveRoot (Contract 6) + test('returns valid result on timeout, not throw', () => { + let threw = false; + let result; + try { + result = resolveWorktreeContext('/tmp', { execGit: makeTimeoutStub() }); + } catch { + threw = true; + } + assert.strictEqual(threw, false, 'must not throw on timeout'); + assert.strictEqual(typeof result, 'object'); 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' + typeof result.effectiveRoot === 'string', + 'must return effectiveRoot string even on timeout' ); }); }); + +// ─── parseWorktreePorcelain ─────────────────────────────────────────────────── + +describe('parseWorktreePorcelain', () => { + test('skips detached HEAD entries', () => { + const porcelain = [ + 'worktree /repo/main', + 'HEAD deadbeef', + 'branch refs/heads/main', + '', + 'worktree /repo/wt-detached', + 'HEAD cafe1234', + 'detached', + '', + 'worktree /repo/wt-feature', + 'HEAD f00dbabe', + 'branch refs/heads/feature-x', + '', + ].join('\n'); + const parsed = parseWorktreePorcelain(porcelain); + assert.deepStrictEqual(parsed, [ + { path: '/repo/main', branch: 'main' }, + { path: '/repo/wt-feature', branch: 'feature-x' }, + ]); + }); +}); + +// ─── planWorktreePrune ──────────────────────────────────────────────────────── + +describe('planWorktreePrune', () => { + test('is non-destructive by default', () => { + const plan = planWorktreePrune('/repo/main', {}, { + execGit: () => ({ exitCode: 0, stdout: 'worktree /repo/main\nbranch refs/heads/main\n', stderr: '' }), + parseWorktreePorcelain: () => [{ path: '/repo/main', branch: 'main' }], + }); + assert.strictEqual(plan.action, 'metadata_prune_only'); + assert.strictEqual(plan.reason, 'worktrees_present'); + assert.strictEqual(plan.destructiveModeRequested, false); + }); + + test('keeps metadata-prune action when destructive mode is requested (scaffold)', () => { + const plan = planWorktreePrune('/repo/main', { allowDestructive: true }, { + execGit: () => ({ exitCode: 0, stdout: '', stderr: '' }), + parseWorktreePorcelain: () => [], + }); + assert.strictEqual(plan.action, 'metadata_prune_only'); + assert.strictEqual(plan.reason, 'no_worktrees'); + assert.strictEqual(plan.destructiveModeRequested, true); + }); + + test('skips when git worktree list fails', () => { + const plan = planWorktreePrune('/repo/main', {}, { + execGit: () => ({ exitCode: 2, stdout: '', stderr: 'fatal' }), + }); + assert.strictEqual(plan.action, 'skip'); + assert.strictEqual(plan.reason, 'git_list_failed'); + }); + + test('still metadata-prunes when porcelain parser throws', () => { + const plan = planWorktreePrune('/repo/main', {}, { + execGit: () => ({ exitCode: 0, stdout: 'not-porcelain', stderr: '' }), + parseWorktreePorcelain: () => { + throw new Error('parse failed'); + }, + }); + assert.strictEqual(plan.action, 'metadata_prune_only'); + assert.strictEqual(plan.reason, 'no_worktrees'); + }); + + // Counter-test: timeout path (Contract 6) + test('returns action=skip when execGit times out', () => { + let threw = false; + let result; + try { + result = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); + } catch { + threw = true; + } + assert.strictEqual(threw, false, 'must not throw on timeout'); + assert.strictEqual(typeof result, 'object'); + assert.strictEqual(result.action, 'skip'); + assert.ok( + typeof result.reason === 'string' && result.reason.length > 0, + 'must return a non-empty reason when git times out' + ); + }); + + // AC4 strict: must use specific reason string 'git_timed_out' + test('reason is git_timed_out (not generic git_list_failed) on timeout', () => { + const result = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); + assert.strictEqual( + result.reason, + 'git_timed_out', + 'must use reason=git_timed_out when execGit returns timedOut:true — not the generic git_list_failed' + ); + }); +}); + +// ─── executeWorktreePrunePlan ───────────────────────────────────────────────── + +describe('executeWorktreePrunePlan', () => { + test('runs git worktree prune for metadata plan', () => { + const calls = []; + const result = executeWorktreePrunePlan( + { repoRoot: '/repo/main', action: 'metadata_prune_only', reason: 'worktrees_present' }, + { + execGit: (args, opts) => { + calls.push({ cwd: opts.cwd, args }); + return { exitCode: 0, stdout: '', stderr: '' }; + }, + } + ); + assert.strictEqual(result.ok, true); + assert.deepStrictEqual(calls, [{ cwd: '/repo/main', args: ['worktree', 'prune'] }]); + }); + + test('returns skip for missing plan', () => { + const result = executeWorktreePrunePlan(null, { + execGit: () => ({ exitCode: 0, stdout: '', stderr: '' }), + }); + assert.strictEqual(result.ok, false); + assert.strictEqual(result.action, 'skip'); + assert.strictEqual(result.reason, 'missing_plan'); + }); + + test('returns skip plan unchanged without git call', () => { + let called = false; + const result = executeWorktreePrunePlan( + { repoRoot: '/repo/main', action: 'skip', reason: 'git_list_failed' }, + { + execGit: () => { + called = true; + return { exitCode: 0, stdout: '', stderr: '' }; + }, + } + ); + assert.strictEqual(result.ok, false); + assert.strictEqual(result.action, 'skip'); + assert.strictEqual(result.reason, 'git_list_failed'); + assert.strictEqual(called, false); + }); + + test('rejects unsupported actions', () => { + const result = executeWorktreePrunePlan( + { repoRoot: '/repo/main', action: 'remove_missing_paths', reason: 'explicit' }, + { + execGit: () => ({ exitCode: 0, stdout: '', stderr: '' }), + } + ); + assert.strictEqual(result.ok, false); + assert.strictEqual(result.action, 'remove_missing_paths'); + assert.strictEqual(result.reason, 'unsupported_action'); + }); + + // Counter-test: timeout path (Contract 6) + test('returns ok:false when plan is skip (timeout path)', () => { + const plan = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); + const result = executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() }); + assert.strictEqual(typeof result, 'object'); + assert.strictEqual(result.ok, false, 'must return ok:false on timeout'); + }); + + // AC4 strict: timedOut must be surfaced as a first-class field + test('result.timedOut is true when prune git call times out', () => { + const plan = { + repoRoot: '/tmp', + action: 'metadata_prune_only', + reason: 'no_worktrees', + destructiveModeRequested: false, + }; + const result = executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() }); + assert.strictEqual(result.ok, false); + assert.strictEqual( + result.timedOut, + true, + 'must include timedOut:true in result when the execGit call returns timedOut:true' + ); + }); +}); + +// ─── listLinkedWorktreePaths ────────────────────────────────────────────────── + +describe('listLinkedWorktreePaths', () => { + test('parses porcelain and skips first/main path', () => { + const listed = listLinkedWorktreePaths('/repo/main', { + execGit: () => ({ + exitCode: 0, + stdout: [ + 'worktree /repo/main', + 'HEAD aaa', + 'branch refs/heads/main', + '', + 'worktree /repo/wt-a', + 'HEAD bbb', + 'branch refs/heads/feat-a', + '', + 'worktree /repo/wt-b', + 'HEAD ccc', + 'detached', + '', + ].join('\n'), + stderr: '', + }), + }); + assert.strictEqual(listed.ok, true); + assert.deepStrictEqual(listed.paths, ['/repo/wt-a', '/repo/wt-b']); + }); + + // Counter-test: failure path (Contract 6) + test('returns ok:false on timeout, not throw', () => { + let threw = false; + let result; + try { + result = listLinkedWorktreePaths('/tmp', { execGit: makeTimeoutStub() }); + } catch { + threw = true; + } + assert.strictEqual(threw, false, 'must not throw on timeout'); + assert.strictEqual(result.ok, false); + assert.ok( + typeof result.reason === 'string' && result.reason.length > 0, + 'must return non-empty reason on timeout' + ); + }); + + test('reason is git_timed_out on timeout', () => { + const result = listLinkedWorktreePaths('/tmp', { execGit: makeTimeoutStub() }); + assert.strictEqual( + result.reason, + 'git_timed_out', + 'must use reason=git_timed_out when execGit returns timedOut:true' + ); + }); +}); + +// ─── inspectWorktreeHealth ──────────────────────────────────────────────────── + +describe('inspectWorktreeHealth', () => { + test('reports orphan and stale findings', () => { + const health = inspectWorktreeHealth( + '/repo/main', + { staleAfterMs: 60 * 60 * 1000, nowMs: 2 * 60 * 60 * 1000 }, + { + execGit: () => ({ + exitCode: 0, + stdout: [ + 'worktree /repo/main', + 'HEAD aaa', + 'branch refs/heads/main', + '', + 'worktree /repo/wt-orphan', + 'HEAD bbb', + 'branch refs/heads/feat-a', + '', + 'worktree /repo/wt-stale', + 'HEAD ccc', + 'branch refs/heads/feat-b', + '', + ].join('\n'), + stderr: '', + }), + existsSync: p => p !== '/repo/wt-orphan', + statSync: () => ({ mtimeMs: 0 }), + } + ); + assert.strictEqual(health.ok, true); + assert.deepStrictEqual(health.findings, [ + { kind: 'orphan', path: '/repo/wt-orphan' }, + { kind: 'stale', path: '/repo/wt-stale', ageMinutes: 120 }, + ]); + }); + + // Counter-test: timeout path (Contract 6) + test('returns ok:false when git times out', () => { + let threw = false; + let result; + try { + result = inspectWorktreeHealth('/tmp', {}, { execGit: makeTimeoutStub() }); + } catch { + threw = true; + } + assert.strictEqual(threw, false, 'must not throw on timeout'); + assert.strictEqual(typeof result, 'object'); + assert.strictEqual(result.ok, false); + }); + + test('findings is empty array (not undefined) on timeout', () => { + const result = inspectWorktreeHealth('/tmp', {}, { execGit: makeTimeoutStub() }); + assert.strictEqual(Array.isArray(result.findings), true, 'findings must be an array even when ok:false'); + }); +}); + +// ─── snapshotWorktreeInventory ──────────────────────────────────────────────── + +describe('snapshotWorktreeInventory', () => { + test('returns typed linked-worktree entries', () => { + const inventory = snapshotWorktreeInventory( + '/repo/main', + { staleAfterMs: 60 * 60 * 1000, nowMs: 2 * 60 * 60 * 1000 }, + { + execGit: () => ({ + exitCode: 0, + stdout: [ + 'worktree /repo/main', + 'HEAD aaa', + 'branch refs/heads/main', + '', + 'worktree /repo/wt-a', + 'HEAD bbb', + 'branch refs/heads/feat-a', + '', + 'worktree /repo/wt-b', + 'HEAD ccc', + 'branch refs/heads/feat-b', + '', + ].join('\n'), + stderr: '', + }), + existsSync: p => p !== '/repo/wt-b', + statSync: () => ({ mtimeMs: 0 }), + } + ); + assert.strictEqual(inventory.ok, true); + assert.deepStrictEqual(inventory.entries, [ + { path: '/repo/wt-a', exists: true, isStale: true, ageMinutes: 120 }, + { path: '/repo/wt-b', exists: false, isStale: false, ageMinutes: null }, + ]); + }); + + // Counter-test: timeout path (Contract 6) + test('returns ok:false with reason on timeout, not throw', () => { + let threw = false; + let result; + try { + result = snapshotWorktreeInventory('/tmp', {}, { execGit: makeTimeoutStub() }); + } catch { + threw = true; + } + assert.strictEqual(threw, false, 'must not throw on timeout'); + assert.strictEqual(typeof result, 'object'); + assert.strictEqual(result.ok, false); + assert.ok( + typeof result.reason === 'string' && result.reason.length > 0, + 'must return non-empty reason on timeout' + ); + }); + + test('reason is git_timed_out on timeout', () => { + const result = snapshotWorktreeInventory('/tmp', {}, { execGit: makeTimeoutStub() }); + assert.strictEqual( + result.reason, + 'git_timed_out', + 'must use reason=git_timed_out when execGit returns timedOut:true' + ); + }); +}); + +// ─── Degraded-git prune flow (AC3) ─────────────────────────────────────────── + +describe('prune flow under degraded git', () => { + test('full prune flow (plan -> execute) completes without throwing on timeout', () => { + let threw = false; + try { + const plan = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() }); + executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() }); + } catch { + threw = true; + } + assert.strictEqual(threw, false, 'full prune flow must not throw on timeout — must degrade gracefully'); + }); +}); + +// ─── planWorktreeWaveCleanup ────────────────────────────────────────────────── + +describe('planWorktreeWaveCleanup', () => { + test('includes only manifest entries and never discovers global agent worktrees', () => { + const plan = planWorktreeWaveCleanup('/repo/main', { + worktrees: [ + { + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'abc123', + }, + ], + }); + assert.equal(plan.ok, true); + assert.deepEqual(plan.entries.map((entry) => ({ + agent_id: entry.agent_id, + worktree_path: entry.worktree_path, + branch: entry.branch, + expected_base: entry.expected_base, + })), [{ + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'abc123', + }]); + assert.equal(plan.discovery, 'manifest'); + }); + + // Counter-test: invalid entries rejected (Contract 6) + test('rejects entries without expected base or disposable branch namespace', () => { + const plan = planWorktreeWaveCleanup('/repo/main', { + worktrees: [ + { + agent_id: 'missing-base', + worktree_path: '/repo/.claude/worktrees/agent-missing-base', + branch: 'worktree-agent-missing-base', + }, + { + agent_id: 'feature-branch', + worktree_path: '/repo/.claude/worktrees/agent-feature', + branch: 'feature/user-work', + expected_base: 'abc123', + }, + ], + }); + assert.equal(plan.ok, false); + assert.equal(plan.reason, 'empty_manifest'); + assert.deepEqual(plan.entries, []); + }); +}); + +// ─── executeWorktreeWaveCleanupPlan ─────────────────────────────────────────── + +describe('executeWorktreeWaveCleanupPlan', () => { + test('does not delete a branch when worktree removal fails', () => { + const calls = []; + const plan = { + ok: true, + repoRoot: '/repo/main', + action: 'cleanup_wave', + discovery: 'manifest', + entries: [{ + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'abc123', + }], + }; + const result = executeWorktreeWaveCleanupPlan(plan, { + execGit: (args, opts) => { + calls.push({ cwd: opts && opts.cwd, args }); + const key = args.join(' '); + if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { + return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; + } + if (key === 'merge-base HEAD worktree-agent-a1') { + return { exitCode: 0, stdout: 'abc123', stderr: '' }; + } + if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key.startsWith('merge worktree-agent-a1')) { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key === 'worktree remove /repo/.claude/worktrees/agent-a1 --force') { + return { exitCode: 1, stdout: '', stderr: 'locked' }; + } + if (key === 'branch -D worktree-agent-a1') { + throw new Error('branch deletion must not run after remove failure'); + } + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }); + assert.equal(result.ok, false); + assert.equal(result.entries[0].status, 'blocked'); + assert.equal(result.entries[0].reason, 'worktree_remove_failed'); + assert.equal(calls.some((call) => call.args.join(' ') === 'branch -D worktree-agent-a1'), false); + }); + + test('stops on merge conflict and records remaining manifest entries', () => { + const plan = { + ok: true, + repoRoot: '/repo/main', + action: 'cleanup_wave', + discovery: 'manifest', + entries: [ + { + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'abc123', + }, + { + agent_id: 'a2', + worktree_path: '/repo/.claude/worktrees/agent-a2', + branch: 'worktree-agent-a2', + expected_base: 'abc123', + }, + ], + }; + const result = executeWorktreeWaveCleanupPlan(plan, { + execGit: (args) => { + const key = args.join(' '); + if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { + return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; + } + if (key === 'merge-base HEAD worktree-agent-a1') { + return { exitCode: 0, stdout: 'abc123', stderr: '' }; + } + if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key.startsWith('merge worktree-agent-a1')) { + return { exitCode: 1, stdout: '', stderr: 'CONFLICT' }; + } + throw new Error(`unexpected git call after conflict: ${key}`); + }, + }); + assert.equal(result.ok, false); + assert.equal(result.entries[0].status, 'blocked'); + assert.equal(result.entries[0].reason, 'merge_failed'); + assert.deepEqual(result.pending.map((entry) => entry.branch), ['worktree-agent-a2']); + }); + + test('blocks dirty worktrees before merge/remove/delete', () => { + const calls = []; + const plan = { + ok: true, + repoRoot: '/repo/main', + action: 'cleanup_wave', + discovery: 'manifest', + entries: [{ + agent_id: 'a1', + worktree_path: '/repo/.claude/worktrees/agent-a1', + branch: 'worktree-agent-a1', + expected_base: 'abc123', + }], + }; + const result = executeWorktreeWaveCleanupPlan(plan, { + execGit: (args) => { + calls.push(args.join(' ')); + const key = args.join(' '); + if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') { + return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' }; + } + if (key === 'merge-base HEAD worktree-agent-a1') { + return { exitCode: 0, stdout: 'abc123', stderr: '' }; + } + if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') { + return { exitCode: 0, stdout: '?? scratch.txt', stderr: '' }; + } + throw new Error(`unexpected git call after dirty check: ${key}`); + }, + }); + assert.equal(result.ok, false); + assert.equal(result.entries[0].reason, 'worktree_dirty'); + assert.equal(calls.some((call) => call.startsWith('merge worktree-agent-a1')), false); + assert.equal(calls.some((call) => call === 'worktree remove /repo/.claude/worktrees/agent-a1 --force'), false); + assert.equal(calls.some((call) => call === 'branch -D worktree-agent-a1'), false); + }); +}); diff --git a/tests/worktree-stagger.test.cjs b/tests/worktree-stagger.test.cjs deleted file mode 100644 index f7e96f21e..000000000 --- a/tests/worktree-stagger.test.cjs +++ /dev/null @@ -1,49 +0,0 @@ -/** - * GSD Worktree Sequential Dispatch Tests - * - * Validates that execute-phase workflow includes sequential dispatch - * instructions to prevent git config.lock contention when multiple - * agents create worktrees in parallel within the same wave. - * - * See: https://github.com/gsd-build/get-shit-done/issues/1511 - */ - -const { test, describe } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -const WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows'); - -describe('worktree sequential dispatch', () => { - const executePhasePath = path.join(WORKFLOWS_DIR, 'execute-phase.md'); - let content; - - test('execute-phase.md exists', () => { - assert.ok(fs.existsSync(executePhasePath), 'execute-phase.md should exist'); - }); - - test('execute-phase explains git config.lock contention', () => { - content = fs.readFileSync(executePhasePath, 'utf-8'); - assert.ok( - content.includes('config.lock'), - 'execute-phase.md should explain the git config.lock race condition' - ); - }); - - test('execute-phase requires sequential dispatch with run_in_background', () => { - content = content || fs.readFileSync(executePhasePath, 'utf-8'); - assert.ok( - content.includes('run_in_background'), - 'execute-phase.md should instruct one-at-a-time dispatch with run_in_background' - ); - }); - - test('execute-phase warns against multiple Task calls in single message', () => { - content = content || fs.readFileSync(executePhasePath, 'utf-8'); - assert.ok( - content.includes('WRONG') && content.includes('single message'), - 'execute-phase.md should warn against sending multiple Task() calls simultaneously' - ); - }); -}); diff --git a/tests/worktree.test.cjs b/tests/worktree.test.cjs new file mode 100644 index 000000000..7f04d5efe --- /dev/null +++ b/tests/worktree.test.cjs @@ -0,0 +1,739 @@ +// allow-test-rule: source-text-is-the-product +// Workflow markdown is the installed orchestration contract. + +'use strict'; + +/** + * Worktree Lifecycle Module — branch-check and workspace-safety tests + * + * Seam: get-shit-done/workflows/{execute-phase,execute-plan,quick}.md, + * agents/gsd-executor.md + * + * Split from the consolidated 13→2 worktree cluster (≤800 LOC/file): + * - tests/bug-2015-worktree-base-branch.test.cjs (#2015: reset --hard) + * - tests/bug-2075-worktree-deletion-safeguards.test.cjs (#2075: git clean prohibition) + * - tests/bug-2431-worktree-locked-surfacing.test.cjs (#2431: locked-worktree errors) + * - tests/bug-2774-worktree-cleanup-workspace-safety.test.cjs (#2774: discovery pipeline) + * + * See also: worktree-cleanup.test.cjs (#2924, #1496, #1756, #1977, #1511, #3384, #3425) + * worktree-safety.test.cjs (safety function unit tests) + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { execSync } = require('child_process'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { cleanup } = require('./helpers.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const EXECUTE_PHASE_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'execute-phase.md'); +const EXECUTE_PLAN_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'execute-plan.md'); +const QUICK_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'quick.md'); +const EXECUTOR_AGENT_PATH = path.join(REPO_ROOT, 'agents', 'gsd-executor.md'); +const DIAGNOSE_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'diagnose-issues.md'); +const GIT_INTEGRATION_PATH = path.join(REPO_ROOT, 'get-shit-done', 'references', 'git-integration.md'); + +const isWindows = process.platform === 'win32'; + +// ─── Helpers ────────────────────────────────────────────────────────────────── + +function extractNamedBlock(markdown, blockName) { + const open = `<${blockName}>`; + const close = ``; + const start = markdown.indexOf(open); + if (start === -1) return null; + const end = markdown.indexOf(close, start + open.length); + if (end === -1) return null; + return markdown.slice(start + open.length, end); +} + +/** + * Extract all fenced code blocks (```...```) from a markdown chunk. + * Returns array of { lang, body } objects. + */ +function extractFencedCodeBlocks(markdown) { + const blocks = []; + const lines = markdown.split('\n'); + let inFence = false; + let fenceLang = ''; + let buffer = []; + for (const line of lines) { + const trimmed = line.trimStart(); + if (trimmed.startsWith('```')) { + if (!inFence) { + inFence = true; + fenceLang = trimmed.slice(3).trim(); + buffer = []; + } else { + blocks.push({ lang: fenceLang, body: buffer.join('\n') }); + inFence = false; + fenceLang = ''; + buffer = []; + } + } else if (inFence) { + buffer.push(line); + } + } + return blocks; +} + +/** + * Tokenize a shell-like script into individual statements (split on `;`, `&&`, `||`, newlines) + * and return commands as arrays of word tokens. Handles `$(cmd ...)` command substitution + * and `VAR=$(cmd ...)` assignments by extracting the inner command. This is intentionally + * simple — adequate for asserting on the presence of well-known git invocations. + */ +function shellStatements(script) { + const statements = []; + const lines = script.split('\n'); + for (let raw of lines) { + const line = raw.replace(/#.*$/, '').trim(); + if (!line) continue; + // Split on shell statement separators + const parts = line.split(/(?:&&|\|\||;)/); + for (const part of parts) { + let trimmed = part.trim(); + if (!trimmed) continue; + // Strip leading `VAR=` assignments so the substituted command surfaces as cmd[0]. + // Then unwrap `$(...)` command substitution. + const assignMatch = trimmed.match(/^[A-Za-z_][A-Za-z0-9_]*=(.*)$/); + if (assignMatch) trimmed = assignMatch[1]; + const subMatch = trimmed.match(/^\$\((.*?)\)?$/); + if (subMatch) trimmed = subMatch[1]; + // Also handle leading `$(` without closing paren (paren may have been split off) + if (trimmed.startsWith('$(')) trimmed = trimmed.slice(2); + // Strip trailing closing parens left over from substitution + trimmed = trimmed.replace(/\)+\s*$/, '').trim(); + if (!trimmed) continue; + // Strip surrounding quotes on the leading word + statements.push(trimmed.split(/\s+/).filter(Boolean)); + } + } + return statements; +} + +/** + * Find the line index of the first command matching a predicate. + * Returns -1 when not found. + */ +function findCommandIndex(statements, predicate) { + for (let i = 0; i < statements.length; i++) { + if (predicate(statements[i])) return i; + } + return -1; +} + + +const DISCOVERY_PIPELINE = + 'grep "^worktree " | grep "\\.claude/worktrees/agent-" | sed \'s/^worktree //\''; + +function runDiscoveryAgainstFixture(porcelain) { + const out = execSync(DISCOVERY_PIPELINE, { + input: porcelain, + encoding: 'utf-8', + }); + return out.split('\n').filter((l) => l.length > 0); +} + +function runDiscoveryAgainstRepo(repoCwd) { + const out = execSync( + `git worktree list --porcelain | ${DISCOVERY_PIPELINE}`, + { cwd: repoCwd, encoding: 'utf-8' } + ); + return out.split('\n').filter((l) => l.length > 0); +} + +function makeTempUpstreamRepo(prefix) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + execSync('git init -b main', { cwd: tmpDir, stdio: 'pipe' }); + execSync('git config user.email "test@test.com"', { cwd: tmpDir, stdio: 'pipe' }); + execSync('git config user.name "Test"', { cwd: tmpDir, stdio: 'pipe' }); + execSync('git config commit.gpgsign false', { cwd: tmpDir, stdio: 'pipe' }); + fs.writeFileSync(path.join(tmpDir, 'README.md'), '# upstream\n'); + execSync('git add -A', { cwd: tmpDir, stdio: 'pipe' }); + execSync('git commit -m "initial"', { cwd: tmpDir, stdio: 'pipe' }); + return tmpDir; +} + +// ─── #2015: reset --hard not --soft ───────────────────────────────────────── + +describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', () => { + + test('execute-phase.md worktree_branch_check does not use reset --soft', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + + // Extract the worktree_branch_check block + const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'execute-phase.md must contain a block'); + + const block = blockMatch[1]; + assert.ok( + !block.includes('reset --soft'), + 'worktree_branch_check must not use reset --soft (leaves working tree files unchanged). Use reset --hard instead.' + ); + }); + + test('execute-phase.md worktree_branch_check uses reset --hard for base correction', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'execute-phase.md must contain a block'); + + const block = blockMatch[1]; + assert.ok( + block.includes('reset --hard'), + 'worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree to the expected base' + ); + }); + + test('quick.md worktree_branch_check does not use reset --soft', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'quick.md must contain a block'); + + const block = blockMatch[1]; + assert.ok( + !block.includes('reset --soft'), + 'quick.md worktree_branch_check must not use reset --soft. Use reset --hard instead.' + ); + }); + + test('quick.md worktree_branch_check uses reset --hard for base correction', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok(blockMatch, 'quick.md must contain a block'); + + const block = blockMatch[1]; + assert.ok( + block.includes('reset --hard'), + 'quick.md worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree' + ); + }); +}); + +// ─── #2075: worktree deletion safeguards ──────────────────────────────────── + +describe('bug-2075: worktree deletion safeguards', () => { + + describe('Failure Mode B: git clean prohibition in executor agent', () => { + test('gsd-executor.md explicitly prohibits git clean in worktree context', () => { + const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); + + // Must have an explicit prohibition section mentioning git clean + const prohibitsGitClean = ( + content.includes('git clean') && + ( + /NEVER.*git clean/i.test(content) || + /git clean.*NEVER/i.test(content) || + /do not.*git clean/i.test(content) || + /git clean.*prohibited/i.test(content) || + /prohibited.*git clean/i.test(content) || + /forbidden.*git clean/i.test(content) || + /git clean.*forbidden/i.test(content) || + /must not.*git clean/i.test(content) || + /git clean.*must not/i.test(content) + ) + ); + + assert.ok( + prohibitsGitClean, + 'gsd-executor.md must explicitly prohibit git clean — running it inside a worktree deletes files committed on the feature branch (#2075 Failure Mode B)' + ); + }); + + test('gsd-executor.md git clean prohibition explains the worktree data-loss risk', () => { + const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); + + // The prohibition must be accompanied by a reason — not just a bare rule + // Look for the word "worktree" near the git clean prohibition + const gitCleanIdx = content.indexOf('git clean'); + assert.ok(gitCleanIdx > -1, 'gsd-executor.md must mention git clean (to prohibit it)'); + + // Extract context around the git clean mention (500 chars either side) + const contextStart = Math.max(0, gitCleanIdx - 500); + const contextEnd = Math.min(content.length, gitCleanIdx + 500); + const context = content.slice(contextStart, contextEnd); + + const hasWorktreeRationale = ( + /worktree/i.test(context) || + /delete/i.test(context) || + /untracked/i.test(context) + ); + + assert.ok( + hasWorktreeRationale, + 'The git clean prohibition in gsd-executor.md must explain why: git clean in a worktree deletes files that appear untracked but are committed on the feature branch' + ); + }); + }); + + describe('Failure Mode A: worktree_branch_check audit across all worktree-spawning workflows', () => { + test('execute-phase.md has worktree_branch_check block with --hard reset', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + + const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok( + blockMatch, + 'execute-phase.md must contain a block' + ); + + const block = blockMatch[1]; + assert.ok( + block.includes('reset --hard'), + 'execute-phase.md worktree_branch_check must use git reset --hard (not --soft)' + ); + assert.ok( + !block.includes('reset --soft'), + 'execute-phase.md worktree_branch_check must not use git reset --soft' + ); + }); + + test('quick.md has worktree_branch_check block with --hard reset', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + + const blockMatch = content.match(/([\s\S]*?)<\/worktree_branch_check>/); + assert.ok( + blockMatch, + 'quick.md must contain a block' + ); + + const block = blockMatch[1]; + assert.ok( + block.includes('reset --hard'), + 'quick.md worktree_branch_check must use git reset --hard (not --soft)' + ); + assert.ok( + !block.includes('reset --soft'), + 'quick.md worktree_branch_check must not use git reset --soft' + ); + }); + + test('diagnose-issues.md has worktree_branch_check instruction for spawned agents', () => { + const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); + + assert.ok( + content.includes('worktree_branch_check'), + 'diagnose-issues.md must include worktree_branch_check instruction for spawned debug agents' + ); + + assert.ok( + content.includes('reset --hard'), + 'diagnose-issues.md worktree_branch_check must instruct agents to use git reset --hard' + ); + }); + }); + + describe('Defense-in-depth: post-commit deletion check (from #1977)', () => { + test('gsd-executor.md task_commit_protocol has post-commit deletion verification', () => { + const content = fs.readFileSync(EXECUTOR_AGENT_PATH, 'utf-8'); + + assert.ok( + content.includes('--diff-filter=D'), + 'gsd-executor.md must include --diff-filter=D to detect accidental file deletions after each commit' + ); + + // Must have a warning about unexpected deletions + assert.ok( + content.includes('DELETIONS') || content.includes('WARNING'), + 'gsd-executor.md must emit a warning when a commit includes unexpected file deletions' + ); + }); + }); + + describe('Defense-in-depth: pre-merge deletion check (from #1977)', () => { + test('execute-phase.md worktree merge section has pre-merge deletion check', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + + const worktreeCleanupStart = content.indexOf('Worktree cleanup'); + assert.ok( + worktreeCleanupStart > -1, + 'execute-phase.md must have a worktree cleanup section' + ); + + const cleanupSection = content.slice(worktreeCleanupStart); + + assert.ok( + cleanupSection.includes('--diff-filter=D'), + 'execute-phase.md worktree cleanup must use --diff-filter=D to block deletion-introducing merges' + ); + + // Deletion check must appear before git merge + const deletionCheckIdx = cleanupSection.indexOf('--diff-filter=D'); + const gitMergeIdx = cleanupSection.indexOf('git merge'); + assert.ok( + deletionCheckIdx < gitMergeIdx, + '--diff-filter=D deletion check must appear before git merge in the worktree cleanup section' + ); + + assert.ok( + cleanupSection.includes('BLOCKED') || cleanupSection.includes('deletion'), + 'execute-phase.md must block or warn when the worktree branch contains file deletions' + ); + }); + + test('quick.md worktree merge section has pre-merge deletion check', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + + const mergeIdx = content.indexOf('git merge'); + assert.ok(mergeIdx > -1, 'quick.md must contain a git merge operation'); + + // Find the worktree cleanup block (starts after "Worktree cleanup") + const worktreeCleanupStart = content.indexOf('Worktree cleanup'); + assert.ok( + worktreeCleanupStart > -1, + 'quick.md must have a worktree cleanup section' + ); + + const cleanupSection = content.slice(worktreeCleanupStart); + + assert.ok( + cleanupSection.includes('--diff-filter=D') || cleanupSection.includes('diff-filter'), + 'quick.md worktree cleanup must check for file deletions before merging' + ); + }); + }); + +}); + +// ─── #2431: locked-worktree error surfacing ────────────────────────────────── + +describe('bug-2431: worktree teardown must surface locked-worktree errors', () => { + test('quick.md exists', () => { + assert.ok(fs.existsSync(QUICK_PATH), 'quick.md should exist'); + }); + + test('execute-phase.md exists', () => { + assert.ok(fs.existsSync(EXECUTE_PHASE_PATH), 'execute-phase.md should exist'); + }); + + test('quick.md: no silent worktree remove pattern', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + const silentRemovePattern = /git worktree remove[^\n]*--force\s+2>\/dev\/null\s*\|\|\s*true/; + assert.ok(!silentRemovePattern.test(content), 'quick.md: must not contain silent git worktree remove pattern'); + }); + + test('execute-phase.md: no silent worktree remove pattern', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + const silentRemovePattern = /git worktree remove[^\n]*--force\s+2>\/dev\/null\s*\|\|\s*true/; + assert.ok(!silentRemovePattern.test(content), 'execute-phase.md: must not contain silent git worktree remove pattern'); + }); + + test('quick.md: has lock-aware detection block', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + assert.ok( + content.includes('.git/worktrees/') && content.includes('locked'), + 'quick.md: must include lock-aware detection (.git/worktrees/.../locked check)' + ); + }); + + test('execute-phase.md: has lock-aware detection block', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok( + content.includes('.git/worktrees/') && content.includes('locked'), + 'execute-phase.md: must include lock-aware detection' + ); + }); + + test('quick.md: has git worktree unlock retry', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + assert.ok(content.includes('git worktree unlock'), 'quick.md: must include "git worktree unlock" retry attempt'); + }); + + test('execute-phase.md: has git worktree unlock retry', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok(content.includes('git worktree unlock'), 'execute-phase.md: must include "git worktree unlock" retry attempt'); + }); + + test('quick.md: has user-visible warning on residual worktree', () => { + const content = fs.readFileSync(QUICK_PATH, 'utf-8'); + assert.ok( + content.includes('Residual worktree') || content.includes('manual cleanup'), + 'quick.md: must include user-visible warning when worktree removal fails' + ); + }); + + test('execute-phase.md: has user-visible warning on residual worktree', () => { + const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8'); + assert.ok( + content.includes('Residual worktree') || content.includes('manual cleanup'), + 'execute-phase.md: must include user-visible warning when worktree removal fails' + ); + }); +}); + +// ─── #2774: cleanup pipeline workspace safety ──────────────────────────────── + +describe('bug #2774 — worktree cleanup pipeline must not target the parent workspace', () => { + describe('discovery pipeline (unit)', () => { + test('selects only the agent worktree when workspace itself is a worktree', () => { + // Fixture mirrors the multi-workspace setup: upstream main + sibling + // workspace worktree + agent worktree under workspace's + // `.claude/worktrees/agent-` namespace. + const porcelain = [ + 'worktree /Users/dev/upstream/get-shit-done', + 'HEAD abc123', + 'branch refs/heads/main', + '', + 'worktree /Users/dev/workspaces/feature-x', + 'HEAD def456', + 'branch refs/heads/workspace/feature-x', + '', + 'worktree /Users/dev/workspaces/feature-x/.claude/worktrees/agent-deadbeef', + 'HEAD 789abc', + 'branch refs/heads/worktree-agent-deadbeef', + '', + ].join('\n'); + + const discovered = runDiscoveryAgainstFixture(porcelain); + + assert.deepEqual( + discovered, + ['/Users/dev/workspaces/feature-x/.claude/worktrees/agent-deadbeef'], + 'pipeline must select only the agent-spawned worktree, never the ' + + 'workspace or upstream main repo' + ); + }); + + test('selects nothing when no agent worktrees exist', () => { + const porcelain = [ + 'worktree /Users/dev/upstream/get-shit-done', + 'HEAD abc123', + 'branch refs/heads/main', + '', + 'worktree /Users/dev/workspaces/feature-x', + 'HEAD def456', + 'branch refs/heads/workspace/feature-x', + '', + ].join('\n'); + + const discovered = runDiscoveryAgainstFixture(porcelain); + + assert.deepEqual(discovered, []); + }); + + test('selects multiple agent worktrees and excludes non-agent paths', () => { + const porcelain = [ + 'worktree /repo/main', + 'HEAD a', + 'branch refs/heads/main', + '', + 'worktree /repo/main/.claude/worktrees/agent-aaa', + 'HEAD b', + 'branch refs/heads/agent-aaa', + '', + 'worktree /repo/main/.claude/worktrees/agent-bbb', + 'HEAD c', + 'branch refs/heads/agent-bbb', + '', + 'worktree /repo/main/some-other-dir', + 'HEAD d', + 'branch refs/heads/feature', + '', + ].join('\n'); + + const discovered = runDiscoveryAgainstFixture(porcelain); + + assert.deepEqual(discovered.sort(), [ + '/repo/main/.claude/worktrees/agent-aaa', + '/repo/main/.claude/worktrees/agent-bbb', + ]); + }); + + test('selects agent worktree even when path contains whitespace', () => { + // Regression for CodeRabbit feedback on PR #2778: `for WT in $WORKTREES` + // splits on whitespace and would emit broken half-paths like + // "/Users/dev/My" and "Workspace/.claude/worktrees/agent-xyz". The + // pipeline output itself is line-delimited and preserves the full path — + // the workflow's loop must consume it line-by-line via `while IFS= read`. + const porcelain = [ + 'worktree /Users/dev/My Workspace', + 'HEAD def456', + 'branch refs/heads/workspace/feature-x', + '', + 'worktree /Users/dev/My Workspace/.claude/worktrees/agent-deadbeef', + 'HEAD 789abc', + 'branch refs/heads/worktree-agent-deadbeef', + '', + ].join('\n'); + + const discovered = runDiscoveryAgainstFixture(porcelain); + + assert.deepEqual( + discovered, + ['/Users/dev/My Workspace/.claude/worktrees/agent-deadbeef'], + 'pipeline output must preserve whitespace-bearing agent worktree path on a single line' + ); + }); + + test('while/read loop iterates each whitespace-bearing path exactly once', + { skip: isWindows ? 'POSIX bash process-substitution `< <(...)` under test; not portable to cmd.exe / git-bash variance' : false }, + () => { + // Verify the actual consumer pattern from quick.md / execute-phase.md: + // while IFS= read -r WT; do ...; done < <() + // Counts the lines yielded to the loop body. With the previous + // `for WT in $WORKTREES` form, a path containing one space would yield + // 2 iterations (broken halves). The `while/read` form yields exactly 1. + const porcelain = [ + 'worktree /tmp/has space/.claude/worktrees/agent-aaa', + 'HEAD a', + 'branch refs/heads/agent-aaa', + '', + 'worktree /tmp/two spaces/.claude/worktrees/agent-bbb', + 'HEAD b', + 'branch refs/heads/agent-bbb', + '', + ].join('\n'); + + // Mirror the workflow's loop verbatim. Print one line per iteration with + // a sentinel so we can count and inspect what the loop actually saw. + const script = ` +while IFS= read -r WT; do + [ -z "$WT" ] && continue + printf 'ITER:%s\\n' "$WT" +done < <(${DISCOVERY_PIPELINE}) +`; + // bash needed for process substitution `< <(...)`. + const out = execSync(`bash -c '${script.replace(/'/g, `'\\''`)}'`, { + input: porcelain, + encoding: 'utf-8', + }); + const iterations = out + .split('\n') + .filter((l) => l.startsWith('ITER:')) + .map((l) => l.slice('ITER:'.length)); + + assert.deepEqual( + iterations, + [ + '/tmp/has space/.claude/worktrees/agent-aaa', + '/tmp/two spaces/.claude/worktrees/agent-bbb', + ], + 'while/read loop must yield exactly one iteration per worktree, with whitespace preserved' + ); + }); + }); + + describe('end-to-end against real git worktrees', + { skip: isWindows ? 'POSIX shell discovery pipeline under test + Windows 8.3 short-name (RUNNER~1) vs long-name path mismatch in temp dirs' : false }, + () => { + let upstream; + let workspace; + let agentWorktree; + let workspacesParent; + + beforeEach(() => { + // Build the multi-worktree scenario from #2774: + // upstream/ <- main repo + // workspace/ <- worktree of upstream (the "workspace") + // workspace/.claude/worktrees/agent-XXXX/ <- agent worktree + upstream = makeTempUpstreamRepo('gsd-2774-upstream-'); + + workspacesParent = fs.mkdtempSync( + path.join(os.tmpdir(), 'gsd-2774-workspaces-') + ); + workspace = path.join(workspacesParent, 'feature-x'); + execSync(`git worktree add -b workspace/feature-x "${workspace}"`, { + cwd: upstream, + stdio: 'pipe', + }); + + const agentDir = path.join(workspace, '.claude', 'worktrees'); + fs.mkdirSync(agentDir, { recursive: true }); + agentWorktree = path.join(agentDir, 'agent-deadbeef'); + execSync( + `git worktree add -b worktree-agent-deadbeef "${agentWorktree}"`, + { cwd: upstream, stdio: 'pipe' } + ); + }); + + afterEach(() => { + try { + execSync('git worktree prune', { cwd: upstream, stdio: 'pipe' }); + } catch (_) { + /* ignore */ + } + cleanup(upstream); + cleanup(workspacesParent); + }); + + test('discovery from inside workspace returns only the agent worktree', () => { + const discovered = runDiscoveryAgainstRepo(workspace); + + // Resolve symlinks (macOS /var → /private/var) for stable comparison. + const expected = fs.realpathSync(agentWorktree); + const actual = discovered.map((p) => fs.realpathSync(p)); + + assert.deepEqual( + actual, + [expected], + 'pipeline must list only the agent worktree, not the workspace or upstream' + ); + }); + + test('running cleanup loop on discovered paths preserves workspace .git', () => { + const workspaceGitBefore = fs.readFileSync( + path.join(workspace, '.git'), + 'utf-8' + ); + assert.ok( + fs.existsSync(path.join(upstream, '.git')), + 'precondition: upstream .git must exist' + ); + + const discovered = runDiscoveryAgainstRepo(workspace); + assert.equal( + discovered.length, + 1, + 'precondition: exactly one agent worktree should be discovered' + ); + + // Execute the cleanup behavior end-to-end: `git worktree remove --force` + // each discovered path. This mirrors the workflow's cleanup loop. + for (const wt of discovered) { + execSync(`git worktree remove --force "${wt}"`, { + cwd: workspace, + stdio: 'pipe', + }); + } + + // Agent worktree dir must be gone. + assert.equal( + fs.existsSync(agentWorktree), + false, + 'agent worktree dir should be removed by cleanup' + ); + + // Workspace `.git` pointer file must still exist and be unchanged — + // the regression we are guarding against. + assert.ok( + fs.existsSync(path.join(workspace, '.git')), + 'workspace .git pointer must survive cleanup (regression #2774)' + ); + assert.equal( + fs.readFileSync(path.join(workspace, '.git'), 'utf-8'), + workspaceGitBefore, + 'workspace .git pointer contents must be unchanged' + ); + + // Upstream repo's .git directory must also be intact. + assert.ok( + fs.existsSync(path.join(upstream, '.git')), + 'upstream .git must survive cleanup' + ); + + // Workspace must still be a functional git worktree. + const branch = execSync('git rev-parse --abbrev-ref HEAD', { + cwd: workspace, + encoding: 'utf-8', + }).trim(); + assert.equal( + branch, + 'workspace/feature-x', + 'workspace must still be a functional worktree on its branch' + ); + }); + }); +}); +