Skip to content

fix(storage): remove Windows worktrees outside their cwd - #2424

Merged
jackwener merged 1 commit into
apache:mainfrom
liugddx:fix/windows-git-worktree-cleanup
Aug 7, 2026
Merged

jackwener merged 1 commit into
apache:mainfrom
liugddx:fix/windows-git-worktree-cleanup

Conversation

@liugddx

@liugddx liugddx commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • execute the final git worktree remove from the stable Git common directory
  • avoid asking a Windows Git process to delete its own current working directory
  • preserve existing clean, detach, Host lease branch deletion, and repository allocation semantics

Closes two failures tracked by #2142:

  • retires only the Host lease branch after a child switches branches
  • recovery preserves live bindings and retires orphaned worktrees

Validation

  • @maka/storage build
  • both affected tests pass in three consecutive Windows runs (6/6)
  • Biome format check
  • git diff --check

The complete test file separately observed a temp-root cleanup EBUSY in an unrelated availability test. This PR does not add broad filesystem retries or claim to address that independent cleanup failure.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — approve & merge

Root cause

runGit uses git -C <cwd> …, and Git chdirs into that path. On Windows, a process cannot delete its own current directory, so:

git -C <worktree> worktree remove --force <worktree>

fails when removing the worktree that is the process cwd.

Fix

Keep clean / detach / lease-branch delete on the worktree (they need the checkout), then run only the final:

git -C <gitCommonDir> worktree remove --force <path>

gitCommonDir is the resolved absolute common dir (realpath of --git-common-dir), already used for repository allocation serialization — outside the worktree being deleted.

Checked

Concern Result
Correct Git context git -C <commonDir> is a valid repo entry point; verified worktree remove from common dir works
Path identity Same absolute path as before; registration path from provision unchanged
Ordering / semantics clean → detach → clean → branch -D → remove preserved
Lease branch only Child-switched branches still preserved (existing test)
Orphan recovery Uses inspected.gitCommonDir; same call path
Allocation lock Still under withRepositoryAllocation(gitCommonDir)
Injection execFile argv array; no shell
Partial failure Pre-existing: branch may be deleted if remove fails mid-way — not introduced
Unix regression Superset behavior; POSIX still fine (cwd restriction is Windows-specific)

Validation

  • CI green including windows_baseline
  • Local: both named tests pass after rebuild

No blocking issues. LGTM.

@jackwener
jackwener merged commit 1c38f2f into apache:main Aug 7, 2026
12 checks passed
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.

2 participants