fleetd/AUDIT.md

Teardown/cleanup audit — dev.ltms.fleet.session

Scope: SessionManager.java, GitWorktrees.java, SessionReaper.java, MemberSession.java, Worktrees.java (interface). Read-only; no code changed.

Main finding

1. SessionManager.java:336-338
2. issue: the final worktree removal in release() is the one step in the whole method
   that is not wrapped in try/catch. Every other cleanup step here (hasUncommitted check,
   snapshot, listener notification) is defended because a `git` call can throw — exec()'s
   own javadoc documents both a non-zero exit and its 30-second timeout as normal failure
   modes, and every sibling worktrees.* call in this class is guarded against exactly that.
   By the time this line runs, registry.remove(paneId) and handles.remove(paneId) have
   already happened and launcher.stop(paneId) has already run, so if worktrees.remove()
   throws here (e.g. `git worktree remove --force` times out on a stale lock file or a
   slow/network filesystem, or exits non-zero), the exception escapes release() with no
   way to retry: the paneId is already gone from the registry, so a second stop call is a
   no-op and never re-attempts the removal. The worktree directory is now leaked forever,
   invisible to `fleet_list`. The caller sees a stop failure — FleetMcp.stop() only catches
   HerdrException, and FleetApp.stopMember() catches nothing — even though the session was
   in fact fully torn down (pane stopped, deregistered, listeners notified).
3. fix: wrap the `worktrees.remove(...)` call at the end of release() in a try/catch that
   logs a warning, matching the pattern already used for every other cleanup step in this
   method (e.g. cleanupAfterAddFailure's own worktree/branch removal, or the dirty-check
   catch above it).
4. severity: medium

Secondary findings

1. SessionManager.java:503-514 (acquireWithWorktree's catch block)
2. issue: after worktrees.add() succeeds, if overlayParity(), shareWithGroup(), or
   launcher.spawn() then throws, the catch block removes only the worktree
   (worktrees.remove(repoRoot, path)) and never deletes the branch `git worktree add`
   created. GitWorktrees.cleanupAfterAddFailure — the sibling cleanup for failures inside
   add() itself — explicitly deletes the branch too, with a `-D` and a documented reason
   ("a branch that never finished provisioning has no session, no PR, nothing else
   pointing at it"). That reasoning applies equally here, but this later catch block (the
   one covering the three post-add() steps) omits it. Since spawn failures are a normal,
   recurring event (this very branch already logs "spawn failed for profile=..."), this
   leaks an orphan `worker/<slug>-<nonce>` branch in the shared repo on every such failure,
   with nothing pointing at it once the (failed) session is never registered.
3. fix: after worktrees.remove(...) succeeds in this catch, also delete the branch with
   `git branch -D branch` (best-effort, log-only on failure), matching
   cleanupAfterAddFailure's own two-step cleanup.
4. severity: low

No other issue in this scope survived a read of every exit of add(), remove(), snapshot(), overlayParity(), shareWithGroup()/shareRootWithGroup(), release(), reapIdle(), drainAll(), and the SessionReaper loop. Two shapes I checked and ruled out as not reachable / not defects:

• release()'s worktrees.repoRoot(removed.cwd()) looked suspicious because cwd for a worktree session is the worktree path itself, so repoRoot would equal worktreePath — but I verified with a live git repo (git --version 2.53.0) that git -C <worktree> worktree remove --force <same worktree> works correctly: git resolves -C against the common git dir regardless of which linked worktree it's given, so this is not a bug.
• git worktree remove --force on a worktree containing a nested .git directory: I expected this to need a double --force per older git docs, but tested it live and a single --force succeeds on git 2.53.0. Not a live failure mode on this stack.

The if (x != null) guard-in-catch shape from fleetd #274 (guard assigned only at the end) does not recur elsewhere in this scope: every catch block that guards on a local now assigns that local before the risky call it protects, not after.