Merge pull request #3739 from jeremymcs/fix/orphaned-worktree-audit

fix(gsd): add orphaned milestone branch audit at auto-mode bootstrap
This commit is contained in:
Jeremy McSpadden 2026-04-07 21:54:57 -05:00 committed by GitHub
commit 2d58b3fc1b
2 changed files with 332 additions and 0 deletions

View file

@ -47,6 +47,10 @@ import {
nativeGetCurrentBranch,
nativeDetectMainBranch,
nativeCheckoutBranch,
nativeBranchList,
nativeBranchListMerged,
nativeBranchDelete,
nativeWorktreeRemove,
} from "./native-git-bridge.js";
import { GitServiceImpl } from "./git-service.js";
import {
@ -56,6 +60,7 @@ import {
} from "./worktree.js";
import { getAutoWorktreePath, isInAutoWorktree } from "./auto-worktree.js";
import { readResourceVersion, cleanStaleRuntimeUnits } from "./auto-worktree.js";
import { worktreePath as getWorktreeDir, isInsideWorktreesDir } from "./worktree-manager.js";
import { initMetrics } from "./metrics.js";
import { initRoutingHistory } from "./routing-history.js";
import { restoreHookState, resetHookState } from "./post-unit-hooks.js";
@ -76,6 +81,7 @@ import {
existsSync,
mkdirSync,
readdirSync,
rmSync,
statSync,
unlinkSync,
} from "node:fs";
@ -117,6 +123,123 @@ export async function openProjectDbIfPresent(basePath: string): Promise<void> {
}
}
/**
* Audit for orphaned milestone branches at bootstrap.
*
* After a milestone completes, the teardown step (merge branch main,
* delete branch, remove worktree) runs as a post-completion engine step.
* If the session ends between completion and teardown, the branch and
* worktree are orphaned the DB says "complete" so auto-mode won't
* re-enter the milestone, and the teardown is never retried.
*
* This audit runs on every fresh bootstrap to catch that gap:
* 1. Lists all local `milestone/*` branches.
* 2. For each, checks if the milestone's DB status is "complete".
* 3. If the branch is already merged into main deletes the branch
* and cleans up any orphaned worktree directory (safe, no data loss).
* 4. If the branch is NOT merged preserves it and warns the user
* so they can merge manually (data safety first).
*
* Returns a summary of actions taken for the caller to surface via notify.
*/
export function auditOrphanedMilestoneBranches(
basePath: string,
isolationMode: "worktree" | "branch" | "none",
): { recovered: string[]; warnings: string[] } {
const recovered: string[] = [];
const warnings: string[] = [];
// Skip in none mode — no milestone branches are created
if (isolationMode === "none") return { recovered, warnings };
// Skip if DB not available — can't determine completion status
if (!isDbAvailable()) return { recovered, warnings };
let milestoneBranches: string[];
try {
milestoneBranches = nativeBranchList(basePath, "milestone/*");
} catch {
// git branch list failed — skip audit
return { recovered, warnings };
}
if (milestoneBranches.length === 0) return { recovered, warnings };
// Detect main branch for merge-check
let mainBranch: string;
try {
mainBranch = nativeDetectMainBranch(basePath);
} catch {
mainBranch = "main";
}
// Get branches already merged into main
let mergedBranches: Set<string>;
try {
mergedBranches = new Set(nativeBranchListMerged(basePath, mainBranch, "milestone/*"));
} catch {
mergedBranches = new Set();
}
for (const branch of milestoneBranches) {
const milestoneId = branch.replace(/^milestone\//, "");
const milestone = getMilestone(milestoneId);
// Only audit completed milestones
if (!milestone || milestone.status !== "complete") continue;
const isMerged = mergedBranches.has(branch);
if (isMerged) {
// Branch is merged — safe to delete branch and clean up worktree dir
try {
nativeBranchDelete(basePath, branch, true);
recovered.push(`Deleted merged branch ${branch} for completed milestone ${milestoneId}.`);
} catch (err) {
warnings.push(`Failed to delete merged branch ${branch}: ${err instanceof Error ? err.message : String(err)}`);
}
// Clean up orphaned worktree directory if it exists
const wtDir = getWorktreeDir(basePath, milestoneId);
if (existsSync(wtDir)) {
// Try git worktree remove first (handles registered worktrees)
try {
nativeWorktreeRemove(basePath, wtDir, true);
} catch (e) {
// Not a registered worktree — expected for orphaned dirs
logWarning("engine", `worktree remove failed (expected for orphaned dirs): ${e instanceof Error ? e.message : String(e)}`);
}
// If the directory still exists after git worktree remove (either it
// wasn't registered or the remove was a noop), fall back to direct
// filesystem removal — but only inside .gsd/worktrees/ for safety (#2365).
if (existsSync(wtDir)) {
if (isInsideWorktreesDir(basePath, wtDir)) {
try {
rmSync(wtDir, { recursive: true, force: true });
recovered.push(`Removed orphaned worktree directory for ${milestoneId}.`);
} catch (err2) {
warnings.push(`Failed to remove worktree directory for ${milestoneId}: ${err2 instanceof Error ? err2.message : String(err2)}`);
}
} else {
warnings.push(`Orphaned worktree directory for ${milestoneId} is outside .gsd/worktrees/ — skipping removal for safety.`);
}
} else {
recovered.push(`Removed orphaned worktree directory for ${milestoneId}.`);
}
}
} else {
// Branch is NOT merged — preserve for safety, warn the user
warnings.push(
`Branch ${branch} exists for completed milestone ${milestoneId} but is NOT merged into ${mainBranch}. ` +
`This may contain unmerged work. Merge manually or run \`/gsd health --fix\` to resolve.`,
);
}
}
return { recovered, warnings };
}
export async function bootstrapAutoSession(
s: AutoSession,
ctx: ExtensionCommandContext,
@ -300,6 +423,26 @@ export async function bootstrapAutoSession(
// derivation (queue-order, task status) works on a cold start (#2841).
await openProjectDbIfPresent(base);
// ── Orphaned milestone branch audit ──
// Catches completed milestones whose teardown (merge + branch delete)
// was lost due to session ending between completion and teardown.
// Must run after DB open and before worktree entry.
try {
const auditResult = auditOrphanedMilestoneBranches(base, getIsolationMode());
for (const msg of auditResult.recovered) {
ctx.ui.notify(`Orphan audit: ${msg}`, "info");
}
for (const msg of auditResult.warnings) {
ctx.ui.notify(`Orphan audit: ${msg}`, "warning");
}
if (auditResult.recovered.length > 0) {
debugLog("orphan-audit", { recovered: auditResult.recovered, warnings: auditResult.warnings });
}
} catch (err) {
// Non-fatal — the audit is defensive, never block bootstrap
logWarning("bootstrap", `orphaned milestone branch audit failed: ${err instanceof Error ? err.message : String(err)}`);
}
let state = await deriveState(base);
// Stale worktree state recovery (#654)

View file

@ -0,0 +1,189 @@
// GSD2 — Tests for auditOrphanedMilestoneBranches bootstrap audit
import { describe, test, beforeEach, afterEach } from "node:test";
import assert from "node:assert/strict";
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync, realpathSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { auditOrphanedMilestoneBranches } from "../auto-start.ts";
import { openDatabase, closeDatabase, insertMilestone, updateMilestoneStatus } from "../gsd-db.ts";
function run(cmd: string, cwd: string): string {
return execSync(cmd, { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" }).trim();
}
/** Create a temp git repo with .gsd structure and DB. */
function createRepo(): string {
const dir = realpathSync(mkdtempSync(join(tmpdir(), "orphan-audit-test-")));
run("git init", dir);
run("git config user.email test@test.com", dir);
run("git config user.name Test", dir);
writeFileSync(join(dir, "README.md"), "# test\n");
run("git add .", dir);
run("git commit -m init", dir);
run("git branch -M main", dir);
// Create .gsd structure on disk (not tracked in git)
mkdirSync(join(dir, ".gsd", "milestones", "M001"), { recursive: true });
return dir;
}
describe("auditOrphanedMilestoneBranches", () => {
let dir: string;
beforeEach(() => {
dir = createRepo();
openDatabase(join(dir, ".gsd", "gsd.db"));
});
afterEach(() => {
closeDatabase();
rmSync(dir, { recursive: true, force: true });
});
test("no milestone branches → no-op", () => {
const result = auditOrphanedMilestoneBranches(dir, "worktree");
assert.deepStrictEqual(result.recovered, []);
assert.deepStrictEqual(result.warnings, []);
});
test("skips in none isolation mode", () => {
// Create a milestone branch that would otherwise be detected
run("git branch milestone/M001", dir);
insertMilestone({ id: "M001", title: "Test", status: "complete" });
const result = auditOrphanedMilestoneBranches(dir, "none");
assert.deepStrictEqual(result.recovered, []);
assert.deepStrictEqual(result.warnings, []);
// Branch should still exist
const branches = run("git branch --list milestone/M001", dir);
assert.ok(branches.includes("milestone/M001"), "branch should be preserved in none mode");
});
test("deletes merged branch for completed milestone", () => {
// Create milestone branch from main (so it's already merged)
run("git branch milestone/M001", dir);
insertMilestone({ id: "M001", title: "Test", status: "complete" });
const result = auditOrphanedMilestoneBranches(dir, "worktree");
assert.ok(result.recovered.length > 0, "should have recovered actions");
assert.ok(
result.recovered.some(r => r.includes("Deleted merged branch milestone/M001")),
"should report branch deletion",
);
assert.deepStrictEqual(result.warnings, []);
// Branch should be gone
const branches = run("git branch --list milestone/M001", dir);
assert.deepStrictEqual(branches, "", "branch should be deleted");
});
test("warns about unmerged branch for completed milestone", () => {
// Create milestone branch with divergent commits (not merged into main)
run("git checkout -b milestone/M001", dir);
writeFileSync(join(dir, "feature.txt"), "new feature\n");
run("git add feature.txt", dir);
run("git commit -m \"add feature on milestone branch\"", dir);
run("git checkout main", dir);
insertMilestone({ id: "M001", title: "Test", status: "complete" });
const result = auditOrphanedMilestoneBranches(dir, "worktree");
assert.deepStrictEqual(result.recovered, [], "should not delete unmerged branch");
assert.ok(result.warnings.length > 0, "should have warnings");
assert.ok(
result.warnings.some(w => w.includes("NOT merged")),
"should warn about unmerged branch",
);
// Branch should still exist (data safety)
const branches = run("git branch --list milestone/M001", dir);
assert.ok(branches.includes("milestone/M001"), "unmerged branch must be preserved");
});
test("skips active (non-complete) milestone branches", () => {
run("git branch milestone/M001", dir);
insertMilestone({ id: "M001", title: "Test", status: "active" });
const result = auditOrphanedMilestoneBranches(dir, "worktree");
assert.deepStrictEqual(result.recovered, []);
assert.deepStrictEqual(result.warnings, []);
// Branch should still exist
const branches = run("git branch --list milestone/M001", dir);
assert.ok(branches.includes("milestone/M001"), "active milestone branch should be preserved");
});
test("cleans up orphaned worktree directory for merged milestone", () => {
// Create milestone branch (merged — same as main)
run("git branch milestone/M001", dir);
insertMilestone({ id: "M001", title: "Test", status: "complete" });
// Create orphaned worktree directory
const wtDir = join(dir, ".gsd", "worktrees", "M001");
mkdirSync(wtDir, { recursive: true });
writeFileSync(join(wtDir, "leftover.txt"), "orphaned file\n");
const result = auditOrphanedMilestoneBranches(dir, "worktree");
assert.ok(result.recovered.length > 0, "should have recovered actions");
assert.ok(
result.recovered.some(r => r.includes("worktree directory")),
"should report worktree cleanup",
);
// Worktree directory should be cleaned up
assert.ok(!existsSync(wtDir), "orphaned worktree directory should be removed");
});
test("handles multiple milestones with mixed states", () => {
// M001: complete, branch merged → should clean up
run("git branch milestone/M001", dir);
insertMilestone({ id: "M001", title: "First", status: "complete" });
// M002: active, branch exists → should skip
run("git branch milestone/M002", dir);
insertMilestone({ id: "M002", title: "Second", status: "active" });
const result = auditOrphanedMilestoneBranches(dir, "worktree");
// M001 should be cleaned up
assert.ok(
result.recovered.some(r => r.includes("M001")),
"should clean up completed M001",
);
// M002 should not be touched
const branches = run("git branch --list milestone/M002", dir);
assert.ok(branches.includes("milestone/M002"), "active M002 branch should be preserved");
});
test("works in branch isolation mode", () => {
run("git branch milestone/M001", dir);
insertMilestone({ id: "M001", title: "Test", status: "complete" });
const result = auditOrphanedMilestoneBranches(dir, "branch");
assert.ok(result.recovered.length > 0, "should work in branch mode too");
assert.ok(
result.recovered.some(r => r.includes("Deleted merged branch")),
"should delete branch in branch mode",
);
});
test("handles milestone in DB but no branch (no-op)", () => {
insertMilestone({ id: "M001", title: "Test", status: "complete" });
const result = auditOrphanedMilestoneBranches(dir, "worktree");
assert.deepStrictEqual(result.recovered, []);
assert.deepStrictEqual(result.warnings, []);
});
});