fix(server): storage cleanup counts a thread's subagents as part of its worktree

[Info-Cado]

Problem

Automatic storage cleanup never removes the worktree of a thread that used a subagent (#16472).

V2 stores every subagent thread (provider-native ones, and delegate_task children) with its parent's worktreePath. cleanWorktrees (apps/server/src/storageCleanup.ts:215-222) only considered a worktree path that exactly one thread owned, and the re-check before removal required latest.length === 1. So any thread with a subagent kept its worktree forever, whatever its age, merge state or status. That rule came from V1 (#11598), where subagents were not app threads. On the reporter's machine, 17 of 23 finished worktrees in one project were shared with 1–12 subagent threads, and all 133 subagent threads with a worktree used their parent's path.

Change

storageCleanupWorktreeOwner(sharers, threadsById, now) decides who owns a checkout:

• Exactly one non-subagent thread may use the path, and it must pass the existing idle check.
• Every other thread on the path must be a subagent whose parent chain reaches that owner through subagent links only. The walk has a cycle guard, and a missing parent fails it.
• Each subagent must not be busy: no active run, an idle/failed status, no pending background task or runtime request, and no queued turn. These are the same checks as before, without the branch requirement.
• A delegated subagent can take messages later. ProviderTurnStartService recreates a missing checkout only from the thread's own branch, so a delegated subagent must have the owner's branch. Provider-native subagents cannot take messages, so their branch: null is fine.
• Any other sharing, such as two top-level threads or a fork, still keeps the worktree.

storageCleanupThreadIdle keeps its behavior. Its activity checks moved into a private storageCleanupThreadBusy so subagents can reuse them. worktreeAfterDays now measures from the latest activity of the owner and its subagents. The re-check before removal runs the same owner rule on a fresh snapshot and compares that activity. The session, terminal, Git, ignored-file and policy checks are unchanged. The storage guide sentence about shared worktrees now covers subagents.

Related, not changed here:

• [Bug]: V2 storage cleanup never frees worktrees for completed/cancelled/interrupted/rolled_back threads #15146 / fix(server): free worktrees for terminal thread statuses #15150: the idle check still rejects completed threads, so most real owners also wait on that fix. fix(server): free worktrees for terminal thread statuses #15150 edits the status line that moved into storageCleanupThreadBusy, so expect a trivial conflict.
• fix(server): storage cleanup reaches archived threads' worktrees #16473: archived threads are missing from the cleanup snapshot on main, so an archived subagent would not be seen here until that lands. That is the same existing gap that already hides archived top-level sharers.

Scope and approval

Fixes #16472. The issue was filed today and is not triaged yet. I am happy to adjust the ownership rule if maintainers prefer another direction.

Verification

• vp test run apps/server/src/storageCleanup.test.ts: 16 passed. New V2 storage cleanup worktree owner cases:
  • single-thread behavior unchanged
  • owner plus idle native, nested and delegated subagents → owner
  • busy native or delegated subagent → kept
  • delegated subagent with null or a different branch → kept
  • subagent with an unknown parent → kept
  • two top-level threads, or another thread's subagent → kept
  • inactivity uses the latest subagent activity
• With the old single-owner rule swapped back in, counts the owner's idle subagents, including nested ones, toward the owner fails.
• tsc --noEmit in apps/server: no errors in the changed files. The only errors are 10 in src/process/externalLauncher.test.ts, which already fail on main at 9bd1d8009a. vp lint and vp fmt on the changed files are clean.
• A Codex review (GPT-6.1-Sol, high) found no blocking issues. I applied its non-blocking notes: doc wording, plus the missing-parent, different-branch and inactivity tests.

Not checked: a live sweep in a running app, and a subagent turning busy between the Git checks and the re-check. The sweep needs Git, settings, terminal and session services, and no test harness covers it today.

Written by Claude Opus 5.5 in T3 Code's Claude Code harness, reviewed by GPT-6.1-Sol in T3 Code's Codex harness.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at 4e53e7f

Macroscope's review found this PR approvable — This is a localized bug fix to existing worktree cleanup, adding conservative subagent ownership and activity handling while preserving the existing filesystem, Git, session, and busy-state safeguards. The accompanying tests cover the new lineage and sharing cases, and documentation is the only other non-test change.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: ef082148-a8c0-4483-95fd-6dd8d10552de

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd1d80 and 4e53e7f.

📒 Files selected for processing (3)

• apps/server/src/storageCleanup.test.ts
• apps/server/src/storageCleanup.ts
• docs/user/project-settings.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Storage cleanup now considers a worktree eligible when one idle non-subagent owner shares it only with eligible idle subagents. Cleanup checks activity across all sharers and revalidates ownership and activity before removal.

Changes

Shared-worktree cleanup

Layer / File(s)	Summary
Determine eligible worktree owners apps/server/src/storageCleanup.ts, apps/server/src/storageCleanup.test.ts	Idle checks account for active runs, thread status, pending tasks and requests, and queued turn starts. Owner eligibility requires exactly one idle non-subagent owner. Other sharers must be idle subagents in that owner’s lineage; delegated subagents must match the owner’s branch, while provider-native subagents bypass that check. Tests cover these rules and related ownership cases.
Select and recheck cleanup candidates apps/server/src/storageCleanup.ts, docs/user/project-settings.md	Live candidates use the owner eligibility check. Age checks and the final recheck use activity across all sharers. Documentation clarifies the exclusions for shared worktrees and busy subagents.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 4e53e

The changed cleanup guard is present, and no actionable merge-blocking issue was established. The change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4e53e

Cleanup remains narrowly constrained by ownership, activity, and filesystem checks. However, a resumed subagent can race with removal of its shared checkout. Existing recovery controls reduce the impact, but do not fully coordinate these operations.

Retained concerns

• Low · reliability · inferred: Newly eligible shared worktrees inherit a checkout-lifetime race. Cleanup holds a workspace lease, but delegated turn startup does not share that lease. A start arriving after the thread and session rechecks can observe the checkout as present before cleanup removes it, potentially failing the resumed turn or removing its cwd while the session opens. Snapshot rechecks, missing-cwd validation, retries, and non-forced removal reduce exposure but do not serialize the transition. The race already affected sole-owner cleanup; this PR expands it to owner-plus-subagent checkouts.

Security review details

Security Blast Radius

• observed — The newly eligible deletion unit is a managed checkout shared by one owner and its eligible subagents. These threads form a common checkout-lifetime failure domain; unrelated owners or subagents outside the owner's ancestry prevent eligibility.

Security Findings and Attack Paths

• inferred — A later delegated turn can overlap the privileged filesystem removal after cleanup's state checks. The supported outcome is a checkout-lifetime or turn-availability failure; no cross-tenant access, credential disclosure, or authorization bypass was established. Shared worktrees were excluded at the base, so their exposure to this inherited race is new.

Trust Boundaries and Controls

• observed — Ownership traversal fails on missing parents and cycles, and must reach the sole owner through subagent links. Every sharing subagent must pass active-run, status, background-task, runtime-request, and queued-turn checks. These controls operate on projected state and do not reserve the checkout against a subsequent start.

Resilience and Maintainability Implications

• observed — Session opening validates that its cwd exists and locks by provider-session identity, not worktree identity. Detected session-open failures are retried when allowed or settle the run as failed. These controls contain detected failures but do not coordinate session opening with cleanup's workspace lease.

Hardening Proposals

• proposed — Coordinate checkout removal and turn startup using the same resolved worktree identity, keeping the checkout reserved until startup's busy or session state is visible to cleanup. This would address the shared checkout-lifetime gap rather than adding another non-atomic snapshot check.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: storage cleanup now counts a thread’s subagents as part of its worktree.
Description check	✅ Passed	The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the ownership rules, links the issue, reports focused test results, and identifies check…
Linked Issues check	✅ Passed	Issue #16472 requires cleanup to treat a thread’s own idle subagents as part of its worktree, while retaining protection for unrelated sharing and delegated threads that cannot recreate the checkout. …
Out of Scope Changes check	✅ Passed	The test changes in apps/server/src/storageCleanup.test.ts, the busy-check refactor and ownership logic in apps/server/src/storageCleanup.ts, and the shared-worktree wording in `docs/user/project-…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}