fix(gsd): add orphaned milestone branch audit at auto-mode bootstrap
When a milestone completes but the session ends before teardown runs, the milestone branch and worktree directory are orphaned — the DB says complete so auto-mode won't re-enter, and the teardown is never retried. Adds auditOrphanedMilestoneBranches() that runs after DB open during bootstrap. For each milestone/* branch where the DB status is complete: - If already merged into main → deletes the branch + cleans worktree dir - If NOT merged → preserves the branch and warns the user Includes 9 regression tests covering merged/unmerged/active/none-mode scenarios.
This commit is contained in:
Jeremy
parent
0dd7c31213
commit
fe8d67579e
2 changed files with 331 additions and 0 deletions
|
|
@ -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";
|
||||
|
|
@ -120,6 +126,122 @@ 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 {
|
||||
// Not a registered worktree — expected for orphaned dirs
|
||||
}
|
||||
|
||||
// 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,
|
||||
|
|
@ -303,6 +425,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)
|
||||
|
|
|
|||
|
|
@ -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, []);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue