diff --git a/extensions/teams/leader.ts b/extensions/teams/leader.ts index a86e2f6..5cd2948 100644 --- a/extensions/teams/leader.ts +++ b/extensions/teams/leader.ts @@ -12,7 +12,7 @@ import { ensureTeamConfig, loadTeamConfig, setMemberStatus, upsertMember, type T import { getTeamDir, getTeamsRootDir } from "./paths.js"; import { assessAttachClaimFreshness, heartbeatTeamAttachClaim, readTeamAttachClaim, releaseTeamAttachClaim } from "./team-attach-claim.js"; import { cleanupTeamDir, gcStaleTeamDirs } from "./cleanup.js"; -import { ensureWorktreeCwd, cleanupWorktrees } from "./worktree.js"; +import { ensureWorktreeCwd, cleanupWorktrees, pruneStaleWorktreeRefs } from "./worktree.js"; import { ActivityTracker, TranscriptTracker } from "./activity-tracker.js"; import { openInteractiveWidget } from "./teams-panel.js"; import { isTeamDone } from "./teams-ui-shared.js"; @@ -115,8 +115,11 @@ async function createSessionForTeammate( } } -/** Check if a team dir has any task files across all task-list namespaces. */ -async function teamDirHasAnyTasks(teamDir: string): Promise { +/** + * Check if a team dir has any pending or in-progress tasks (i.e. active work remaining). + * Returns false when the team has no tasks at all, or only completed tasks. + */ +async function teamDirHasActiveTasks(teamDir: string): Promise { const tasksDir = path.join(teamDir, "tasks"); let taskListDirs: string[]; try { @@ -130,7 +133,23 @@ async function teamDirHasAnyTasks(teamDir: string): Promise { const stat = await fs.promises.stat(listPath); if (!stat.isDirectory()) continue; const files = await fs.promises.readdir(listPath); - if (files.some((f) => f.endsWith(".json") && !f.startsWith("."))) return true; + for (const f of files) { + if (!f.endsWith(".json") || f.startsWith(".")) continue; + try { + const raw = await fs.promises.readFile(path.join(listPath, f), "utf8"); + const parsed: unknown = JSON.parse(raw); + if ( + typeof parsed === "object" && + parsed !== null && + "status" in parsed && + (parsed as Record).status !== "completed" + ) { + return true; + } + } catch { + // Unreadable task file — skip. + } + } } catch { continue; } @@ -746,14 +765,23 @@ export function runLeader(pi: ExtensionAPI): void { }); // Startup GC: silently remove stale team directories from previous sessions (24h age floor). + // Pass repoCwd so cleanupWorktrees can find the repo root even when worktree dirs are gone. void gcStaleTeamDirs({ teamsRootDir: getTeamsRootDir(), maxAgeMs: 24 * 60 * 60 * 1000, + repoCwd: ctx.cwd, excludeTeamIds: new Set([currentTeamId]), }).catch(() => { // Best-effort; never block the session. }); + // Standalone prune: clean up dangling .git/worktrees/ entries from sessions whose + // team directories were already deleted (crash, manual rm, partial cleanup). + // Lightweight — only removes bookkeeping for worktree dirs that no longer exist. + void pruneStaleWorktreeRefs(ctx.cwd).catch(() => { + // Best-effort; never block the session. + }); + await refreshTasks(); renderWidget(); @@ -863,7 +891,6 @@ export function runLeader(pi: ExtensionAPI): void { if (!currentCtx) return; await releaseActiveAttachClaim(currentCtx); stopLoops(); - const hadTeammates = teammates.size > 0; const strings = getTeamsStrings(style); await stopAllTeammates(currentCtx, `The ${strings.teamNoun} is over`); @@ -878,28 +905,26 @@ export function runLeader(pi: ExtensionAPI): void { // Best-effort — don't block shutdown. } - // Exit cleanup: delete own team directory if it's empty. - // Conservative: only if no RPC teammates were active, no online workers in - // config (manual/tmux), no tasks in ANY namespace, and no fresh attach claim. - // (Dirs with completed tasks are left for the 24h startup GC — intentionally - // asymmetric for safety.) - if (!hadTeammates) { - try { - const claim = await readTeamAttachClaim(teamDir); - const claimIsLive = claim !== null && !assessAttachClaimFreshness(claim).isStale; - if (claimIsLive) { - // Another session is using this team — don't delete. - } else { - // Also check config for online non-lead members (manual/tmux workers). - const cfg = await loadTeamConfig(teamDir); - const hasOnlineWorkers = cfg?.members.some((m) => m.role !== "lead" && m.status === "online") ?? false; - if (!hasOnlineWorkers && !(await teamDirHasAnyTasks(teamDir))) { - await cleanupTeamDir(getTeamsRootDir(), teamDir); - } + // Exit cleanup: delete own team directory when safe. + // Safe = no live attach claim, no online non-lead workers, and no active + // (pending/in-progress) tasks. Dirs with only completed tasks are cleaned + // up here rather than deferred to the 24h startup GC — completed tasks have + // already been reported back and serve no ongoing purpose. + try { + const claim = await readTeamAttachClaim(teamDir); + const claimIsLive = claim !== null && !assessAttachClaimFreshness(claim).isStale; + if (!claimIsLive) { + const cfg = await loadTeamConfig(teamDir); + const hasOnlineWorkers = cfg?.members.some((m) => m.role !== "lead" && m.status === "online") ?? false; + if (!hasOnlineWorkers && !(await teamDirHasActiveTasks(teamDir))) { + await cleanupTeamDir(getTeamsRootDir(), teamDir, { + teamId: currentTeamId, + repoCwd: currentCtx.cwd, + }); } - } catch { - // Best-effort; never block shutdown. } + } catch { + // Best-effort; never block shutdown. } } }); diff --git a/extensions/teams/worktree.ts b/extensions/teams/worktree.ts index 845b04e..9645480 100644 --- a/extensions/teams/worktree.ts +++ b/extensions/teams/worktree.ts @@ -244,3 +244,22 @@ export async function cleanupWorktrees(opts: { return { removedWorktrees, removedBranches, warnings }; } + +/** + * Run `git worktree prune` on a repo to clean up stale `.git/worktrees/` entries + * left behind by deleted worktree directories (e.g. from crash, manual deletion, + * or partial cleanup). + * + * This is a lightweight, standalone operation safe to call on every startup. + * It only removes bookkeeping entries whose on-disk worktree directories no longer exist. + */ +export async function pruneStaleWorktreeRefs(repoCwd: string): Promise<{ pruned: boolean; warning?: string }> { + try { + const toplevel = (await execGit(["rev-parse", "--show-toplevel"], { cwd: repoCwd })).stdout.trim(); + if (!toplevel) return { pruned: false }; + await execGit(["worktree", "prune"], { cwd: toplevel, timeoutMs: 15_000 }); + return { pruned: true }; + } catch { + return { pruned: false, warning: "git worktree prune failed (non-fatal)" }; + } +} diff --git a/scripts/integration-cleanup-test.mts b/scripts/integration-cleanup-test.mts index 4e3371f..1d96d1c 100644 --- a/scripts/integration-cleanup-test.mts +++ b/scripts/integration-cleanup-test.mts @@ -15,7 +15,7 @@ import * as os from "node:os"; import { execFileSync } from "node:child_process"; import { cleanupTeamDir, gcStaleTeamDirs, assertTeamDirWithinTeamsRoot } from "../extensions/teams/cleanup.js"; -import { cleanupWorktrees } from "../extensions/teams/worktree.js"; +import { cleanupWorktrees, pruneStaleWorktreeRefs } from "../extensions/teams/worktree.js"; import { ensureTeamConfig } from "../extensions/teams/team-config.js"; import { createTask } from "../extensions/teams/task-store.js"; @@ -393,6 +393,94 @@ console.log("\n10. cleanupWorktrees (no repo context)"); assert(!fs.existsSync(fakePath), "fake-agent dir removed"); } +// ── Test 11: pruneStaleWorktreeRefs cleans orphaned metadata ───── +console.log("\n11. pruneStaleWorktreeRefs"); +{ + // Create a worktree, then delete it from disk without git worktree remove. + // This leaves stale .git/worktrees/ metadata. + const teamId = "team-prune-refs"; + const teamDir = path.join(teamsRoot, teamId); + const wtDir = path.join(teamDir, "worktrees"); + fs.mkdirSync(wtDir, { recursive: true }); + + const shortTeam = teamId.slice(0, 12); + const agentPath = path.join(wtDir, "orphan-agent"); + const branch = `pi-teams/${shortTeam}/orphan-agent`; + + git(["worktree", "add", "-b", branch, agentPath, "HEAD"], repoDir); + + // Verify worktree is listed + const wtBefore = gitLines(["worktree", "list", "--porcelain"], repoDir); + assert(wtBefore.some((l) => l.includes("orphan-agent")), "orphan worktree exists before deletion"); + + // Delete the worktree directory WITHOUT git worktree remove (simulates crash/manual rm) + fs.rmSync(agentPath, { recursive: true, force: true }); + + // Worktree still shows in git metadata (stale entry) + const wtStale = gitLines(["worktree", "list", "--porcelain"], repoDir); + assert(wtStale.some((l) => l.includes("orphan-agent")), "orphan worktree still in git metadata after rm"); + + // pruneStaleWorktreeRefs should clean it up + const pruneResult = await pruneStaleWorktreeRefs(repoDir); + assert(pruneResult.pruned, "pruneStaleWorktreeRefs returned pruned=true"); + assert(pruneResult.warning === undefined, "no warning from pruneStaleWorktreeRefs"); + + // Verify the stale entry is gone + const wtAfter = gitLines(["worktree", "list", "--porcelain"], repoDir); + assert(!wtAfter.some((l) => l.includes("orphan-agent")), "orphan worktree removed from git metadata after prune"); + + // Clean up the branch + try { + git(["branch", "-D", branch], repoDir); + } catch { + // Branch may already be gone + } +} + +// ── Test 12: pruneStaleWorktreeRefs handles non-git dir ────────── +console.log("\n12. pruneStaleWorktreeRefs (non-git dir)"); +{ + const nonGitDir = path.join(tmpRoot, "not-a-repo"); + fs.mkdirSync(nonGitDir, { recursive: true }); + + const result = await pruneStaleWorktreeRefs(nonGitDir); + assert(!result.pruned, "returns pruned=false for non-git dir"); +} + +// ── Test 13: gcStaleTeamDirs removes dirs with only completed tasks +console.log("\n13. gcStaleTeamDirs (completed-only tasks are GC'd)"); +{ + const teamId = "old-completed"; + const teamDir = path.join(teamsRoot, teamId); + fs.mkdirSync(teamDir, { recursive: true }); + await ensureTeamConfig(teamDir, { teamId, taskListId: teamId, leadName: "lead", style: "normal" }); + + // Create a completed task + const task = await createTask(teamDir, teamId, { subject: "done task", description: "finished" }); + const taskFile = path.join(teamDir, "tasks", teamId, `${task.id}.json`); + const taskData = JSON.parse(fs.readFileSync(taskFile, "utf8")); + taskData.status = "completed"; + fs.writeFileSync(taskFile, JSON.stringify(taskData, null, 2)); + + // Backdate + const twoDaysAgoLocal = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000); + const cfg = JSON.parse(fs.readFileSync(path.join(teamDir, "config.json"), "utf8")); + cfg.createdAt = twoDaysAgoLocal.toISOString(); + fs.writeFileSync(path.join(teamDir, "config.json"), JSON.stringify(cfg, null, 2)); + fs.utimesSync(teamDir, twoDaysAgoLocal, twoDaysAgoLocal); + + const result = await gcStaleTeamDirs({ + teamsRootDir: teamsRoot, + maxAgeMs: 24 * 60 * 60 * 1000, + repoCwd: repoDir, + dryRun: false, + }); + + // Dirs with only completed tasks should be removed — no active work. + assert(result.removed.includes(teamId), "gc: old-completed removed (only completed tasks)"); + assert(!fs.existsSync(teamDir), "gc: old-completed dir deleted"); +} + // ── cleanup ────────────────────────────────────────────────────── console.log("\n─── Cleanup ───"); try {