Skip to content

123: Factory leaks per-issue git worktrees: cleanup only removes the current run's dir (14 GB orphaned observed)#151

Merged
kjgbot merged 15 commits into
mainfrom
factory/123-agentworkforce-factory-4bc95c4d
Jul 21, 2026
Merged

123: Factory leaks per-issue git worktrees: cleanup only removes the current run's dir (14 GB orphaned observed)#151
kjgbot merged 15 commits into
mainfrom
factory/123-agentworkforce-factory-4bc95c4d

Conversation

@agent-relay-code

@agent-relay-code agent-relay-code Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Factory leaks per-issue git worktrees under .factory-worktrees/<checkout>/. Completion cleanup only removes the worktree of the currently-tracked dispatch run, so any re-dispatch of the same issue under a new run identifier orphans the previous run's directory forever. On a real operator run these accumulated to 39 orphaned hoopsheet worktrees consuming ~14 GB, all for issues that were already CLOSED with MERGED PRs.

Root cause

Worktree directory names embed a per-dispatch identifier that changes across runs, while cleanup is scoped to a single run:

  1. Dispatch path (src/orchestrator/factory.ts:10097factoryWorktreePath, src/git/agent-worktree.ts:202) names the dir <issueSlug>-<repoSlug>-${runId.slice(0,8)}. runId falls back to a fresh randomUUID() (factory.ts:2575, factory.ts:2717) whenever a durable dispatch lifecycle isn't reloaded — daemon restart, takeover, or state not found for the issue.
  2. Babysitter / existing-PR path (factory.ts:1814-1818) builds the worktree path with a different suffix scheme — stableHash(${pr.repo}#${pr.prNumber}:${headRef}) — so the same issue can get a second worktree at a path the dispatch-path cleanup never references.
  3. Completion cleanup (#cleanupAgentWorktrees, factory.ts:4693) iterates only record.agents of the current in-flight record and removes just that run's spec.clonePath. git worktree prune (called inside GitAgentWorktreeManager.cleanup, src/git/agent-worktree.ts:116,123) only drops admin records for directories that are already gone — it never deletes a live directory.

Net effect: there is no per-issue sweep that removes .factory-worktrees/<checkout>/<issueSlug>-<repoSlug>-* across all stale run identifiers. Every re-dispatch under a new runId (or the babysitter's hash scheme) strands the prior directory.

terminalState: human-review + mergePolicy: never amplify this: issues linger in-flight longer and are re-dispatched / babysat / survived across daemon restarts more often, each restart minting a new randomUUID runId.

Evidence (observed)

  • 39 orphaned worktrees under .factory-worktrees/hoopsheet/, ~14 GB, 0 running hoopsheet agents.
  • Several issues had 3–4 worktrees each (e.g. issue 24 had 4; issues 34/36/37 had 3).
  • Of 5 worktrees with uncommitted changes, 3 had a different hash than their merged PR branch — i.e. abandoned parallel/retried attempts whose sibling run merged — confirming the "new identifier per re-dispatch" mechanism.
  • All affected issues were already CLOSED / MERGED, so every orphan was pure leaked disk.

Proposed changes

A. Per-issue worktree sweep on completion

When an issue reaches its terminal state (#finishDurableRelease#cleanupAgentWorktrees, factory.ts:3082/4693), remove all worktrees matching that issue, not just the tracked run:

  • Glob .factory-worktrees/<sanitize(basename(baseClonePath))>/<issueSlug>-<repoSlug>-* for each routed repo.
  • For each match, run the existing safe GitAgentWorktreeManager.cleanup teardown (git worktree remove --force + git worktree prune + removeEmptyWorktreeParents), reusing assertSafeWorktree so nothing outside .factory-worktrees/<checkout>/ is ever touched.
  • Skip (and log) any match with uncommitted changes or unpushed commits so genuine in-progress work is never destroyed by the sweep.

B. Startup reaper for issues no longer in flight

On daemon start (after in-flight registry load), reap orphans whose issue is not in the current in-flight set:

  • Enumerate .factory-worktrees/<checkout>/* directories.
  • For any whose <issueSlug> is not an active in-flight issue for this workspace, and which is clean (no uncommitted changes, no unpushed commits, no lock), tear it down via the same safe path.
  • log() a one-line summary of how many were reaped and how much was reclaimed. Never silently skip dirty worktrees — surface them so an operator can decide.

Acceptance criteria

  • Completing an issue that was dispatched under multiple run IDs removes every <issue>-<repo>-* worktree for that issue, not just the last run's.
  • The sweep and the reaper never remove a worktree with uncommitted changes, unpushed commits, or a git lock; such worktrees are retained and logged.
  • The sweep/reaper only ever operate on paths under .factory-worktrees/<sanitize(basename(baseClonePath))>/ (reuse assertSafeWorktree); a path outside that root throws rather than deletes.
  • Counters: extend the existing agentWorktreesCleaned / agentWorktreeCleanupFailures (factory.ts:4704,4707) and add a reaped-on-startup counter.
  • Unit tests in src/orchestrator/factory.test.ts and/or src/git/agent-worktree.test.ts:
    • two run IDs for one issue → both dirs removed on completion;
    • a dirty extra-run dir → retained and logged, clean ones removed;
    • startup reaper removes a clean orphan for a not-in-flight issue and retains a dirty one;
    • a crafted path outside .factory-worktrees/<checkout>/ is refused.

Out of scope (follow-ups)

  • Unifying the two worktree-path suffix schemes (dispatch runId vs babysitter stableHash(pr#head)) so an issue always resolves to a single stable worktree path. Worth doing, but the sweep/reaper above fixes the leak regardless.
  • Making .factory-worktrees location or a retention window configurable.

Notes for the implementer

  • Do not change dispatch/branch behavior; this is cleanup-only plus a startup reaper.
  • Reuse GitAgentWorktreeManager.cleanup and assertSafeWorktree — do not hand-roll rm -rf.
  • Run npm test (vitest); add the tests above.
  • Manual repro: dispatch an issue, restart the daemon so a fresh runId is minted, let it complete, and confirm the first run's .factory-worktrees/<checkout>/<issue>-<repo>-<oldhash> directory is gone.

Fixes #123


Summary by cubic

Stops leaking per-issue git worktrees by sweeping all runs on completion and reaping clean orphans on startup. Adds symlink-proof root/container checks, strict realpath/registration validation, pre-cleanup inspection (git locks, uncommitted/unpushed), and reclaimed-size reporting; addresses Linear #123.

  • New Features

    • Startup reaper (non dispatch-owner) excludes active issues from durable lifecycles, waiting clarifications, and online legacy agents; reaps orphans (including stale legacy registries) and logs reclaimed size.
    • Repository-wide discovery: listWorktrees + factoryWorktreeIssueSlug find all <issue>-<repo>-* runs across configured checkouts for completion cleanup and startup reaping.
    • Pre-cleanup inspectForCleanup returns bytes and retentionReasons; new ports AgentWorktreeRepository and AgentWorktreeCleanupInspection.
  • Bug Fixes

    • Symlink-proof safety: reject symbolic-link roots/containers, resolve realpaths, and forbid targets outside /.factory-worktrees/<checkout>/.
    • Never delete local work: retain and warn when retentionReasons exist; counters updated (agentWorktreesCleaned, agentWorktreeCleanupRetained, agentWorktreesReapedOnStartup, agentWorktreeCleanupFailures).
    • Prune stale git worktree registrations before inspection; treat unreadable registrations and repository enumeration failures as non-fatal so one bad entry can’t block sibling cleanup.

Written for commit 994abbf. Summary will update on new commits.

Review in cubic

Copy link
Copy Markdown
Member

Babysitter update: PR #151 is green and ready for human review at e37ff98.

I merged the current main (a099034) and tightened two cleanup safety edges found during the audit: the startup reaper now preserves issues represented by nonterminal durable lifecycles, waiting clarifications, or the legacy registry, and worktree cleanup now rejects symlink paths whose real target escapes the Factory worktree root.

Validation completed:

  • npm run build
  • npm run featuremap:check
  • npm test — 53 files / 1,076 tests passed
  • npm run verify:e2e — 9 checks passed, head-bound to e37ff98
  • npm pack --dry-run
  • Fresh-head GitHub package CI and Cubic review checks passed

There are no open review threads or requested changes. Happy to discuss the active-lifecycle retention guard, cleanup safety trade-offs, or any other part of the implementation. Per policy, I have not merged the PR.

@kjgbot
kjgbot merged commit 3a8d828 into main Jul 21, 2026
2 checks passed
@kjgbot
kjgbot deleted the factory/123-agentworkforce-factory-4bc95c4d branch July 21, 2026 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Factory leaks per-issue git worktrees: cleanup only removes the current run's dir (14 GB orphaned observed)

2 participants