Skip to content

fix: resilient worktree removal on Windows file-lock (#302) - #308

Merged
getappz merged 3 commits into
masterfrom
task/308
Jul 22, 2026
Merged

getappz merged 3 commits into
masterfrom
task/308

Conversation

@getappz

@getappz getappz commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

Supersedes #306, which is stuck in a CONFLICTING state — its branch (feat/worktree-audit-294) still carries the pre-squash commit history of the already-merged #294/#304, diverging from master's squashed version at the git-object level (no actual content conflict, just incompatible history). Rebuilding cleanly on current master instead of fighting that.

Cherry-picked the one genuinely new commit from that branch and fixed two gaps found in review:

  • fmt: one eprintln! wasn't rustfmt-clean — CI's fmt job would have failed had it run (it never did on fix: resilient worktree removal on Windows file-lock (#302) #306, blocked by the CONFLICTING status; only CLA/CodeQL/CodeRabbit reported).
  • Tests: the new remove_worktree_dir (retry + backoff, cmd /c rmdir fallback, handle64.exe/handle.exe locking-process detection) had zero test coverage. Added two: a happy-path removal, and a genuine lock-then-release test (opens a file handle, drops it from a background thread after 250ms, asserts the retry loop succeeds within budget) — meaningful on Windows where an open handle actually blocks remove_dir_all; harmless no-op assertion on Unix where it doesn't.

Changes (from item #302)

  • remove_worktree_dir() in gc_orphans: retry remove_dir_all with exponential backoff (100ms-1.6s, ~3.1s total + one final attempt), fall back to cmd /c rmdir /s /q on Windows, then try handle64.exe/handle.exe (Sysinternals, if present — fails gracefully via Command's Err path if not installed) to report the locking process.
  • cleanup-branches.sh: retry git worktree remove up to 5x with backoff, distinguish a genuine lock (Permission denied) from dirty/uncommitted (never retried, still skipped immediately), warn and fall through on a persistent lock instead of blocking the rest of the cleanup run.

Test plan

  • cargo test -p flare-git-core — 98/98 pass (2 new)
  • cargo fmt --all -- --check — clean
  • cargo clippy -p flare-git-core --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic — clean
  • bash -n scripts/cleanup-branches.sh — syntax valid (shellcheck not available in this environment)

Note for reviewers

cmd /c rmdir receives the worktree path as a Command::args() element (not shell-concatenated), but cmd.exe itself still parses %...%/&/| inside whatever it receives. Not fixed here — worktree paths in this codebase are always internally constructed from repo_root + a numeric sequence_id/branch-derived name, never external/untrusted input, so real-world exploitability is effectively nil. Flagging for the record rather than adding defensive escaping for a threat model that doesn't apply.

gc_orphans: retry remove_dir_all with exponential backoff, fall back to
cmd /c rmdir, detect locking process via handle64.exe.

cleanup-branches.sh: retry git worktree remove on Permission denied with
backoff; distinguish dirty vs locked; warn and fall through on lock so
branch cleanup is not blocked.

Item #302: git worktree remove --force .worktrees/ fails with Permission
denied on Windows when rust-analyzer holds file handles.

Agentflare-Agent: 1
Agentflare-Branch: feat/worktree-audit-294
@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@getappz, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 20797eaa-850e-4822-9df1-0fed832e5759

📥 Commits

Reviewing files that changed from the base of the PR and between 921b472 and 888be54.

📒 Files selected for processing (2)
  • crates/flare-git-core/src/worktree.rs
  • scripts/cleanup-branches.sh
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/308

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

getappz added 2 commits July 22, 2026 21:17
Agentflare-Agent: claude-code_2-1-217_agent
Agentflare-Branch: task/308
Agentflare-Item: 308
Agentflare-Agent: claude-code_2-1-217_agent
Agentflare-Branch: task/308
Agentflare-Item: 308
@getappz
getappz enabled auto-merge (squash) July 22, 2026 15:54
@getappz
getappz merged commit 26b2dac into master Jul 22, 2026
16 checks passed
@getappz
getappz deleted the task/308 branch July 22, 2026 15:59
getappz added a commit that referenced this pull request Oct 6, 2026
…dow log, argv-error cooldown (#308) (#870)

* wip(sdd-loop): task 0 checkpoint

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308

* docs+fmt: document failover metadata/config keys, fix cargo fmt drift

Adds an "Automatic failover" section to the `agentflare work` CLI docs
covering the `"failover"`/`"allowed_agents"` item-metadata keys and the
`[failover]` config.toml table (usage_threshold_percent, five_hour_percent,
seven_day_percent), per item #308's acceptance criteria. Also runs
`cargo fmt` to clean up formatting drift left in the prior checkpoint.

Agentflare-Agent: claude-code
Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308
Agentflare-Session: 31ba6896-9f51-43f4-9062-92c8e073cfc6

* fix: preserve branches when worktree cleanup fails

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308
Agentflare-Session: bb396fc0-d442-4982-b50b-b58e444833e4

* test: create cleanup fixture worktree from HEAD

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308

* Review completed; fixture fix 2749048 and shell checks passed. Outstanding: partial worktree deletion on removal failure and branch mismatch.
Review issues remain: partial worktree deletion on permission failure; fixture fix 2749048 is on task/308 rather than the named PR branch.
Commit 082215e skips checked-out worktrees and preserves loose-branch cleanup; focused test, Bash syntax, and diff checks passed. Review of scope and branch mismatch pending.
Partial deletion path addressed and focused test passed. Commits 2749048 and 082215e remain on task/308; feat/worktree-audit-294 is absent locally.
Safety fix 082215e passed the focused test. PR #306 is closed; feat/worktree-audit-294 is absent locally and remotely. Await corrected PR or branch target.
Safety fix 082215e and focused test passed on task/308. PR #306 is closed; feat/worktree-audit-294 is absent locally and remotely. Await corrected target.
Code fix 082215e is committed and the focused test passes. Stop retrying this task until a corrected PR or branch target is supplied.
Stop fix and review rounds. Tested fix 082215e is on task/308; a corrected PR or branch target is required.
Park PR-target mismatch pending a corrected target. Fix 082215e remains committed and tested on the claimed task/308 branch.
Skip stale PR #306 task. Preserve tested commits 2749048 and 082215e on task/308 for a correctly targeted follow-up.

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308

* fix(failover): validate thresholds and narrow argv failures

---------

Co-authored-by: shiva <shiva@gosysinfo.tech>
getappz added a commit that referenced this pull request Oct 6, 2026
… report review_result (#882)

* wip(sdd-loop): task 0 checkpoint

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308

* docs+fmt: document failover metadata/config keys, fix cargo fmt drift

Adds an "Automatic failover" section to the `agentflare work` CLI docs
covering the `"failover"`/`"allowed_agents"` item-metadata keys and the
`[failover]` config.toml table (usage_threshold_percent, five_hour_percent,
seven_day_percent), per item #308's acceptance criteria. Also runs
`cargo fmt` to clean up formatting drift left in the prior checkpoint.

Agentflare-Agent: claude-code
Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308
Agentflare-Session: 31ba6896-9f51-43f4-9062-92c8e073cfc6

* fix: preserve branches when worktree cleanup fails

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308
Agentflare-Session: bb396fc0-d442-4982-b50b-b58e444833e4

* test: create cleanup fixture worktree from HEAD

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308

* Review completed; fixture fix 2749048 and shell checks passed. Outstanding: partial worktree deletion on removal failure and branch mismatch.
Review issues remain: partial worktree deletion on permission failure; fixture fix 2749048 is on task/308 rather than the named PR branch.
Commit 082215e skips checked-out worktrees and preserves loose-branch cleanup; focused test, Bash syntax, and diff checks passed. Review of scope and branch mismatch pending.
Partial deletion path addressed and focused test passed. Commits 2749048 and 082215e remain on task/308; feat/worktree-audit-294 is absent locally.
Safety fix 082215e passed the focused test. PR #306 is closed; feat/worktree-audit-294 is absent locally and remotely. Await corrected PR or branch target.
Safety fix 082215e and focused test passed on task/308. PR #306 is closed; feat/worktree-audit-294 is absent locally and remotely. Await corrected target.
Code fix 082215e is committed and the focused test passes. Stop retrying this task until a corrected PR or branch target is supplied.
Stop fix and review rounds. Tested fix 082215e is on task/308; a corrected PR or branch target is required.
Park PR-target mismatch pending a corrected target. Fix 082215e remains committed and tested on the claimed task/308 branch.
Skip stale PR #306 task. Preserve tested commits 2749048 and 082215e on task/308 for a correctly targeted follow-up.

Agentflare-Branch: task/308-usage-threshold-failover-treats-70-weekl
Agentflare-Item: 308

* fix(failover): validate thresholds and narrow argv failures

---------

Co-authored-by: shiva <shiva@gosysinfo.tech>
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.

1 participant