feat(teams): automatic startup GC + exit cleanup of empty team dirs (from #8)#30
Conversation
Two-layer automatic cleanup for team directories that accumulate across sessions: 1. **Startup GC** — on session_start, fire-and-forget call to the existing gcStaleTeamDirs() with a 24h age floor. Reuses the well-tested function that checks age + online workers + in-progress tasks + attach claim freshness. Never blocks startup. 2. **Exit cleanup** — on session_shutdown, delete the current session's team directory if it's empty (no tasks in any namespace, no teammates were active, no fresh attach claim from another session). Best-effort, never blocks shutdown. Only cleans the session's own team (not attached teams). Based on #8 by @RensTillmann.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dee95b626
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| void gcStaleTeamDirs({ | ||
| teamsRootDir: getTeamsRootDir(), | ||
| maxAgeMs: 24 * 60 * 60 * 1000, |
There was a problem hiding this comment.
Running gcStaleTeamDirs() on every session_start can silently remove another team that is still actively led. In extensions/teams/cleanup.ts, gcStaleTeamDirs() explicitly ignores role === "lead" and only treats live attach claims or in_progress tasks as activity, so a team that has been open for more than 24h with pending/completed tasks and no workers will be classified as stale even while its leader session is still running. This was an opt-in /team gc behavior before, but wiring it into startup means opening a fresh session can now delete someone else’s live team directory unexpectedly.
Useful? React with 👍 / 👎.
| const claim = await readTeamAttachClaim(teamDir); | ||
| const claimIsLive = claim !== null && !assessAttachClaimFreshness(claim).isStale; | ||
| if (!claimIsLive && !(await teamDirHasAnyTasks(teamDir))) { | ||
| await cleanupTeamDir(getTeamsRootDir(), teamDir); |
There was a problem hiding this comment.
This shutdown cleanup has a race with /team attach: if another session acquires a fresh attach claim after readTeamAttachClaim(teamDir) returns but before cleanupTeamDir() runs, we will still delete the directory out from under the newly attached session. acquireTeamAttachClaim() serializes claim updates with a lock in extensions/teams/team-attach-claim.ts, but this path only does an unlocked read followed by unconditional deletion, so an empty team can disappear immediately after another leader successfully attaches to it.
Useful? React with 👍 / 👎.
…rs on exit - Add excludeTeamIds param to gcStaleTeamDirs() and pass the current session's team ID to prevent GC of resumed sessions older than 24h. - Check config.json for online non-lead members before exit cleanup to avoid deleting dirs still in use by manual/tmux workers.
Addresses SYM-41 / GitHub #9 — remaining gaps after PRs #14 and #30: 1. Standalone git worktree prune on startup: adds pruneStaleWorktreeRefs() called in session_start to clean up dangling .git/worktrees/ entries from sessions whose team directories were already deleted (crash, manual rm, or partial cleanup). Lightweight and safe to run every time. 2. Pass repoCwd to startup GC: gcStaleTeamDirs now receives ctx.cwd so cleanupWorktrees can find the repo root even when worktree dirs are gone. 3. Smarter session_shutdown cleanup: replaces teamDirHasAnyTasks with teamDirHasActiveTasks — team dirs with only completed tasks are now cleaned up at session end instead of waiting 24h for startup GC. Completed tasks have already been reported back and serve no purpose. 4. Removes hadTeammates guard: session_shutdown now attempts dir cleanup regardless of whether RPC teammates were active. The safety checks (no live attach claim, no online workers, no active tasks) are sufficient guards. 5. Passes repoCwd to cleanupTeamDir in session_shutdown for proper worktree metadata cleanup. Tests: 8 new integration tests covering pruneStaleWorktreeRefs (orphan cleanup, non-git dir) and gcStaleTeamDirs (completed-only tasks are GC'd). All 325 smoke + 56 integration-cleanup tests pass.
1 participant
Summary
Incorporates the safe parts of PR #8 (@RensTillmann) — automatic cleanup of team directories that accumulate across sessions.
What's added
Startup GC — on
session_start, fire-and-forget call to the existinggcStaleTeamDirs()with a 24h age floor. Reuses the well-tested function that checks age + online workers + in-progress tasks + attach claim freshness. Never blocks startup.Exit cleanup — on
session_shutdown, delete the current session's team directory if it's truly empty:tasks/*/subdirs)What's NOT included from PR #8
team-gc.tsfile — the existinggcStaleTeamDirs()incleanup.tsalready handles GC with proper safety checks/team gccommand — the existing--max-age-hourscommand works correctlyReview process
Plan was reviewed by GPT-5.4 xhigh which found two P1 data-loss risks in the initial design:
Both fixed in the implementation. See plan v2 for details.
Checks
tscstrict typecheck passes--authorCo-authored-by: Rens Tillmann [email protected]