From 37ec08d23c8e5d61de37fda7fd5fba2a99de98dc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 15:36:01 -0400 Subject: [PATCH] fix: prevent UAT stuck-loop and orphaned worktree after milestone completion (#3065) Bug 1 -- UAT stuck-loop: syncProjectRootToWorktree used force:false for all milestone files, which preserved stale ASSESSMENT files in the worktree. When the project root had a passing verdict but the worktree retained a FAIL copy (or lost it during DB rebuild), checkNeedsRunUat found no passing verdict and re-dispatched run-uat indefinitely (x9). Fix: after the additive-only safeCopyRecursive, walk ASSESSMENT files in the project root and force-overwrite the worktree copy when the source contains a verdict field. This is safe because ASSESSMENT verdicts are only ever overwritten in a forward direction (FAIL -> PASS on retry). Bug 2 -- Orphaned worktree: removeWorktree silently swallowed failures from git worktree remove when untracked files (UAT-RESULT, ASSESSMENT) blocked removal. The .git/worktrees/ internal directory held a lock that also prevented the rmSync fallback from working. Fix: after both native removal attempts fail, explicitly remove the git internal .git/worktrees/ directory first, then retry rmSync on the worktree filesystem directory. Log a warning with manual cleanup instructions if the final attempt also fails. Closes #2821 Co-authored-by: Claude Opus 4.6 --- src/resources/extensions/gsd/auto-worktree.ts | 74 +++++ .../uat-stuck-loop-orphaned-worktree.test.ts | 289 ++++++++++++++++++ .../extensions/gsd/worktree-manager.ts | 23 ++ 3 files changed, 386 insertions(+) create mode 100644 src/resources/extensions/gsd/tests/uat-stuck-loop-orphaned-worktree.test.ts diff --git a/src/resources/extensions/gsd/auto-worktree.ts b/src/resources/extensions/gsd/auto-worktree.ts index c07c7d4e5..f86e4f6bc 100644 --- a/src/resources/extensions/gsd/auto-worktree.ts +++ b/src/resources/extensions/gsd/auto-worktree.ts @@ -102,6 +102,67 @@ function isSamePath(a: string, b: string): boolean { } } +// ─── ASSESSMENT Force-Sync Helper (#2821) ───────────────────────────────── + +/** Regex matching YAML frontmatter `verdict:` field. */ +const VERDICT_RE = /verdict:\s*[\w-]+/i; + +/** + * Walk a milestone directory and force-overwrite ASSESSMENT files in the + * destination when the source copy contains a `verdict:` field. + * + * This is the targeted fix for the UAT stuck-loop (#2821): the main + * safeCopyRecursive uses force:false to protect worktree-authoritative + * files (#1886), but ASSESSMENT files written by run-uat must be + * forward-synced when the project root has a verdict. Without this, + * the worktree retains a stale FAIL or missing ASSESSMENT and + * checkNeedsRunUat re-dispatches run-uat indefinitely. + * + * Only overwrites when the source has a verdict — never clobbers a + * worktree ASSESSMENT with a verdictless project-root copy. + */ +function forceOverwriteAssessmentsWithVerdict( + srcMilestoneDir: string, + dstMilestoneDir: string, +): void { + if (!existsSync(srcMilestoneDir)) return; + + // Walk slices// looking for *-ASSESSMENT.md files + const slicesDir = join(srcMilestoneDir, "slices"); + if (!existsSync(slicesDir)) return; + + try { + for (const sliceEntry of readdirSync(slicesDir, { withFileTypes: true })) { + if (!sliceEntry.isDirectory()) continue; + const srcSliceDir = join(slicesDir, sliceEntry.name); + const dstSliceDir = join(dstMilestoneDir, "slices", sliceEntry.name); + + try { + for (const fileEntry of readdirSync(srcSliceDir, { withFileTypes: true })) { + if (!fileEntry.isFile()) continue; + if (!fileEntry.name.endsWith("-ASSESSMENT.md")) continue; + + const srcFile = join(srcSliceDir, fileEntry.name); + try { + const srcContent = readFileSync(srcFile, "utf-8"); + if (!VERDICT_RE.test(srcContent)) continue; // no verdict in source — skip + + // Source has a verdict — force-copy into worktree + mkdirSync(dstSliceDir, { recursive: true }); + safeCopy(srcFile, join(dstSliceDir, fileEntry.name), { force: true }); + } catch { + /* non-fatal per file */ + } + } + } catch { + /* non-fatal per slice */ + } + } + } catch { + /* non-fatal */ + } +} + // ─── Module State ────────────────────────────────────────────────────────── /** Original project root before chdir into auto-worktree. */ @@ -214,6 +275,19 @@ export function syncProjectRootToWorktree( { force: false }, ); + // Force-sync ASSESSMENT files that have a verdict from project root (#2821). + // The additive-only copy above preserves worktree-authoritative files, but + // ASSESSMENT files are special: after run-uat writes a verdict and post-unit + // syncs it to the project root, the worktree may retain a stale copy (e.g. + // verdict:fail while the project root has verdict:pass from a retry). On + // session resume the DB is rebuilt from disk, and if the stale ASSESSMENT + // persists, checkNeedsRunUat finds no passing verdict → re-dispatches + // run-uat indefinitely (stuck-loop ×9). + forceOverwriteAssessmentsWithVerdict( + join(prGsd, "milestones", milestoneId), + join(wtGsd, "milestones", milestoneId), + ); + // Forward-sync completed-units.json from project root to worktree. // Project root is authoritative for completion state after crash recovery; // without this, the worktree re-dispatches already-completed units (#1886). diff --git a/src/resources/extensions/gsd/tests/uat-stuck-loop-orphaned-worktree.test.ts b/src/resources/extensions/gsd/tests/uat-stuck-loop-orphaned-worktree.test.ts new file mode 100644 index 000000000..44ae79661 --- /dev/null +++ b/src/resources/extensions/gsd/tests/uat-stuck-loop-orphaned-worktree.test.ts @@ -0,0 +1,289 @@ +/** + * uat-stuck-loop-orphaned-worktree.test.ts — Regression tests for #2821. + * + * Reproduces two cascading bugs: + * + * Bug 1 — UAT stuck-loop: syncProjectRootToWorktree uses force:false for + * milestone files. When the project root has an ASSESSMENT with a verdict + * but the worktree has a stale/empty ASSESSMENT (or none at all after DB + * rebuild), the verdict is NOT synced into the worktree. checkNeedsRunUat + * finds no verdict → re-dispatches run-uat indefinitely. + * + * Bug 2 — Orphaned worktree: removeWorktree silently swallows failures when + * git worktree remove fails (untracked files, CWD inside worktree, etc.). + * The worktree directory and branch persist on disk after teardown. + * teardownAutoWorktree has a fallback rmSync but it also fails when the + * git internal .git/worktrees/ directory holds a lock. + */ + +import { describe, test, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { + mkdtempSync, + mkdirSync, + writeFileSync, + rmSync, + existsSync, + readFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { execFileSync } from "node:child_process"; + +import { syncProjectRootToWorktree } from "../auto-worktree.ts"; +import { + createWorktree, + removeWorktree, + worktreePath, +} from "../worktree-manager.ts"; + +function git(args: string[], cwd: string): string { + return execFileSync("git", args, { + cwd, + stdio: ["ignore", "pipe", "pipe"], + encoding: "utf-8", + }).trim(); +} + +function makeBaseRepo(): string { + const base = mkdtempSync(join(tmpdir(), "gsd-2821-")); + git(["init", "-b", "main"], base); + git(["config", "user.name", "Test"], base); + git(["config", "user.email", "test@test.com"], base); + writeFileSync(join(base, "README.md"), "# test\n"); + mkdirSync(join(base, ".gsd", "milestones", "M011"), { recursive: true }); + git(["add", "."], base); + git(["commit", "-m", "init"], base); + return base; +} + +// ─── Bug 1: ASSESSMENT force-sync ───────────────────────────────────────── + +describe("#2821 Bug 1 — ASSESSMENT file force-synced on resume", () => { + let mainBase: string; + let wtBase: string; + + beforeEach(() => { + mainBase = mkdtempSync(join(tmpdir(), "gsd-2821-main-")); + wtBase = mkdtempSync(join(tmpdir(), "gsd-2821-wt-")); + mkdirSync(join(mainBase, ".gsd", "milestones", "M011", "slices", "S01"), { + recursive: true, + }); + mkdirSync(join(wtBase, ".gsd", "milestones", "M011", "slices", "S01"), { + recursive: true, + }); + }); + + afterEach(() => { + rmSync(mainBase, { recursive: true, force: true }); + rmSync(wtBase, { recursive: true, force: true }); + }); + + test("force-syncs ASSESSMENT with verdict from project root into worktree when worktree copy has no verdict", () => { + // Project root has ASSESSMENT with a PASS verdict (written by run-uat, synced by post-unit) + const prAssessment = join( + mainBase, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ); + writeFileSync( + prAssessment, + "---\nverdict: pass\n---\n# S01 Assessment\nAll tests pass.\n", + ); + + // Worktree has a stale ASSESSMENT with FAIL verdict (from the initial run-uat execution) + const wtAssessment = join( + wtBase, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ); + writeFileSync( + wtAssessment, + "---\nverdict: fail\n---\n# S01 Assessment\nSome tests fail.\n", + ); + + syncProjectRootToWorktree(mainBase, wtBase, "M011"); + + // The worktree ASSESSMENT must now have the project root's PASS verdict + const content = readFileSync(wtAssessment, "utf-8"); + assert.ok( + content.includes("verdict: pass"), + `Expected worktree ASSESSMENT to have verdict:pass after sync, got: ${content.slice(0, 100)}`, + ); + }); + + test("force-syncs ASSESSMENT from project root when worktree has no ASSESSMENT at all", () => { + // Project root has ASSESSMENT with verdict + const prAssessment = join( + mainBase, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ); + writeFileSync( + prAssessment, + "---\nverdict: pass\n---\n# S01 Assessment\n", + ); + + // Worktree has NO ASSESSMENT (deleted during DB rebuild) + // — file simply doesn't exist + + syncProjectRootToWorktree(mainBase, wtBase, "M011"); + + const wtAssessment = join( + wtBase, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ); + assert.ok( + existsSync(wtAssessment), + "ASSESSMENT should be copied to worktree when missing", + ); + const content = readFileSync(wtAssessment, "utf-8"); + assert.ok( + content.includes("verdict: pass"), + `Synced ASSESSMENT should contain verdict:pass, got: ${content.slice(0, 100)}`, + ); + }); + + test("does NOT overwrite worktree ASSESSMENT when project root has no verdict", () => { + // Project root has ASSESSMENT without verdict (incomplete) + const prAssessment = join( + mainBase, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ); + writeFileSync(prAssessment, "# S01 Assessment\nIn progress...\n"); + + // Worktree has ASSESSMENT with verdict:fail + const wtAssessment = join( + wtBase, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ); + writeFileSync( + wtAssessment, + "---\nverdict: fail\n---\n# S01 Assessment\nSome tests fail.\n", + ); + + syncProjectRootToWorktree(mainBase, wtBase, "M011"); + + // Worktree copy should NOT be overwritten by the verdictless project root copy + const content = readFileSync(wtAssessment, "utf-8"); + assert.ok( + content.includes("verdict: fail"), + `Worktree ASSESSMENT should keep verdict:fail when project root has no verdict, got: ${content.slice(0, 100)}`, + ); + }); +}); + +// ─── Bug 2: Orphaned worktree cleanup ───────────────────────────────────── + +describe("#2821 Bug 2 — removeWorktree cleans up despite untracked files", () => { + let base: string; + + beforeEach(() => { + base = makeBaseRepo(); + }); + + afterEach(() => { + rmSync(base, { recursive: true, force: true }); + }); + + test("removes worktree directory even when it contains untracked files", () => { + const info = createWorktree(base, "M011", { + branch: "milestone/M011", + }); + + // Simulate run-uat writing untracked files (S01-UAT-RESULT.md, ASSESSMENT) + mkdirSync( + join(info.path, ".gsd", "milestones", "M011", "slices", "S01"), + { recursive: true }, + ); + writeFileSync( + join( + info.path, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-UAT-RESULT.md", + ), + "# UAT Result\nverdict: fail\n", + ); + writeFileSync( + join( + info.path, + ".gsd", + "milestones", + "M011", + "slices", + "S01", + "S01-ASSESSMENT.md", + ), + "---\nverdict: fail\n---\n# Assessment\n", + ); + + removeWorktree(base, "M011", { + branch: "milestone/M011", + deleteBranch: true, + force: true, + }); + + const wtDir = worktreePath(base, "M011"); + assert.ok( + !existsSync(wtDir), + `Worktree directory should be removed after teardown, but still exists at ${wtDir}`, + ); + }); + + test("removes git internal worktree metadata after filesystem removal", () => { + createWorktree(base, "M011", { + branch: "milestone/M011", + }); + + removeWorktree(base, "M011", { + branch: "milestone/M011", + deleteBranch: true, + force: true, + }); + + // The git internal worktree directory should be cleaned up + const gitInternalWorktreeDir = join(base, ".git", "worktrees", "M011"); + assert.ok( + !existsSync(gitInternalWorktreeDir), + `Git internal worktree dir should be removed: ${gitInternalWorktreeDir}`, + ); + + // The branch should be deleted + const branches = git(["branch"], base); + assert.ok( + !branches.includes("milestone/M011"), + "milestone/M011 branch should be deleted after removeWorktree", + ); + }); +}); diff --git a/src/resources/extensions/gsd/worktree-manager.ts b/src/resources/extensions/gsd/worktree-manager.ts index 5cf93e387..16a943a33 100644 --- a/src/resources/extensions/gsd/worktree-manager.ts +++ b/src/resources/extensions/gsd/worktree-manager.ts @@ -365,6 +365,29 @@ export function removeWorktree( try { nativeWorktreeRemove(basePath, resolvedWtPath, true); } catch { /* may fail */ } } + // (#2821) If the worktree directory STILL exists after both native removal + // attempts (e.g. untracked files like ASSESSMENT/UAT-RESULT prevent git + // worktree remove), force-remove the git internal worktree metadata first, + // then remove the filesystem directory. Without this, the .git/worktrees/ + // lock prevents rmSync from cleaning up, and the orphaned worktree directory + // causes every subsequent `/gsd auto` to re-enter the stale worktree. + if (existsSync(resolvedWtPath)) { + try { + const wtInternalDir = join(basePath, ".git", "worktrees", name); + if (existsSync(wtInternalDir)) { + rmSync(wtInternalDir, { recursive: true, force: true }); + } + rmSync(resolvedWtPath, { recursive: true, force: true }); + } catch { + logWarning( + "reconcile", + `Worktree directory could not be removed after git internal cleanup: ${resolvedWtPath}. ` + + `Manual cleanup: rm -rf "${resolvedWtPath.replaceAll("\\", "/")}"`, + { worktree: name }, + ); + } + } + // Prune stale entries so git knows the worktree is gone nativeWorktreePrune(basePath);