Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 50 additions & 25 deletions extensions/teams/leader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<boolean> {
/**
* 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<boolean> {
const tasksDir = path.join(teamDir, "tasks");
let taskListDirs: string[];
try {
Expand All @@ -130,7 +133,23 @@ async function teamDirHasAnyTasks(teamDir: string): Promise<boolean> {
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<string, unknown>).status !== "completed"
) {
return true;
}
} catch {
// Unreadable task file — skip.
Comment on lines +149 to +150
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat unreadable task JSON as active during shutdown cleanup

teamDirHasActiveTasks now skips task files that fail readFile/JSON.parse, so a team with malformed or partially written task JSON can be misclassified as having no active tasks; in session_shutdown that allows cleanupTeamDir(...) to delete the entire team directory and lose unresolved task data. The previous teamDirHasAnyTasks logic was fail-closed for this case, so this change introduces a new data-loss path when task files are corrupted or in an unexpected format.

Useful? React with 👍 / 👎.

}
}
} catch {
continue;
}
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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`);

Expand All @@ -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.
}
}
});
Expand Down
19 changes: 19 additions & 0 deletions extensions/teams/worktree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)" };
}
}
90 changes: 89 additions & 1 deletion scripts/integration-cleanup-test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 {
Expand Down
Loading