fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

#309: clean partial worktrees after add failure #312

Closed
agent wants to merge 0 commits from worker/fix-309-ec3939-8 into main
pull from: worker/fix-309-ec3939-8
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-04 08:20:00 +02:00
Member
Copy Link
Copy Source

Fixes #309.

Summary

Moves git worktree add into GitWorktrees.add cleanup scope. The original WorktreeException still reaches the caller. Cleanup stays best-effort.

The cleanup now returns quietly when the generated worktree path does not exist. This keeps an ordinary Git refusal quiet and avoids deleting an existing branch. The javadoc now describes the partial-state risk instead of claiming atomicity.

Tests

  • mvn -Dtest=GitWorktreesTest test: Tests run: 51, Failures: 0, Errors: 0, Skipped: 0; BUILD SUCCESS.
  • Mutation proof: moving the add runner outside the try failed with: the worktree directory leaked after the add runner failed ==> expected: <false> but was: <true>. I restored the fix.
  • cd fleetd && mvn clean install: Tests run: 1318, Failures: 0, Errors: 0, Skipped: 0; BUILD SUCCESS.

Test design and caveat

The new narrow add-runner seam performs a real git worktree add in a JUnit temporary repository, then throws a synthetic timeout-shaped WorktreeException. It proves cleanup removes the real worktree and branch without waiting for a slow checkout. It does not observe an actual 30-second timeout or process interruption. The temporary repo does not have this repository’s submodule, --skip-worktree file, or per-worktree config. It covers Git worktree and branch cleanup only.

Search and issue check

I searched fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java, fleetd/src/main/java/dev/ltms/fleet/session/SessionManager.java, and fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java for GitWorktrees, cleanupAfterAddFailure, and .add(. The production call site is SessionManager.acquireWithWorktree at path = worktrees.add(...); it leaves path null when add throws.

The issue matches the code. One extra risk appeared while checking invariant 3: the old unconditional cleanup would call deleteBranch after an ordinary existing-branch failure. The fix skips cleanup when the generated worktree path was never created, and a test confirms the existing branch remains and no warning is logged. This is a resource leak, not data loss. No real timeout has been observed.

Fixes #309. ## Summary Moves `git worktree add` into `GitWorktrees.add` cleanup scope. The original `WorktreeException` still reaches the caller. Cleanup stays best-effort. The cleanup now returns quietly when the generated worktree path does not exist. This keeps an ordinary Git refusal quiet and avoids deleting an existing branch. The javadoc now describes the partial-state risk instead of claiming atomicity. ## Tests - `mvn -Dtest=GitWorktreesTest test`: `Tests run: 51, Failures: 0, Errors: 0, Skipped: 0`; `BUILD SUCCESS`. - Mutation proof: moving the add runner outside the try failed with: `the worktree directory leaked after the add runner failed ==> expected: <false> but was: <true>`. I restored the fix. - `cd fleetd && mvn clean install`: `Tests run: 1318, Failures: 0, Errors: 0, Skipped: 0`; `BUILD SUCCESS`. ## Test design and caveat The new narrow add-runner seam performs a real `git worktree add` in a JUnit temporary repository, then throws a synthetic timeout-shaped `WorktreeException`. It proves cleanup removes the real worktree and branch without waiting for a slow checkout. It does not observe an actual 30-second timeout or process interruption. The temporary repo does not have this repository’s submodule, `--skip-worktree` file, or per-worktree config. It covers Git worktree and branch cleanup only. ## Search and issue check I searched `fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java`, `fleetd/src/main/java/dev/ltms/fleet/session/SessionManager.java`, and `fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java` for `GitWorktrees`, `cleanupAfterAddFailure`, and `.add(`. The production call site is `SessionManager.acquireWithWorktree` at `path = worktrees.add(...)`; it leaves `path` null when add throws. The issue matches the code. One extra risk appeared while checking invariant 3: the old unconditional cleanup would call `deleteBranch` after an ordinary existing-branch failure. The fix skips cleanup when the generated worktree path was never created, and a test confirms the existing branch remains and no warning is logged. This is a resource leak, not data loss. No real timeout has been observed.
agent added 1 commit 2026-09-04 08:20:01 +02:00
#309: clean partial worktrees after add failure
CI / contract (pull_request) Successful in 1m23s
Details
CI / build (pull_request) Successful in 2m2s
Details
002329adb5
ltms closed this pull request 2026-09-04 08:31:06 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m23s
Details
CI / build (pull_request) Successful in 2m2s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#312
Delete Branch "worker/fix-309-ec3939-8"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?