From 9ee6760dbb97122bea728bc126a7532e535d96a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 15:36:24 +0000 Subject: [PATCH 1/4] Retry transient failures when deleting worktrees and branches Worktree/branch deletion sometimes fails for reasons that clear on their own: on Windows a worktree file is still held open by an editor or antivirus scan, a racing git process leaves a ref/index .lock, or a network blip interrupts deleting the branch on origin. Previously any of these aborted the cleanup and left a half-tidied branch. Wrap each git delete step (worktree remove, local branch delete, remote branch delete) in a Polly resilience pipeline that retries only transient-looking failures a few times with backoff, narrating each retry in the flight log. Permanent refusals ("use --force", "remote ref does not exist", "not fully merged") are classified as non-transient and still fail fast on the first attempt, so behaviour and tests for those paths are unchanged. - New GitRetry: transient-failure predicate + the retry pipeline, isolated so it can be unit-tested without git. - GitService gains an injectable GitCommandRunner seam so tests can script a transient-then-success git result end-to-end. - Polly.Core (Native-AOT/trim-safe) added; verified a clean AOT publish with zero trim warnings. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01KfYxaFc4xQHTt75dhdQoWx --- CHANGELOG.md | 5 +- Docs/Features.md | 7 +- src/Fido.csproj | 4 + src/Services/GitRetry.cs | 164 ++++++++++++++++++ src/Services/GitService.cs | 14 +- src/Services/OpenerService.cs | 33 +++- tests/Fido.Tests/Services/GitRetryTests.cs | 128 ++++++++++++++ .../Services/OpenerDeletionRetryTests.cs | 123 +++++++++++++ 8 files changed, 467 insertions(+), 11 deletions(-) create mode 100644 src/Services/GitRetry.cs create mode 100644 tests/Fido.Tests/Services/GitRetryTests.cs create mode 100644 tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c70139..b19a638 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the ticked targets — **removing the linked worktree, deleting the local branch, and deleting the branch on `origin`**. The work runs from the clone's main tree (so the worktree is dropped cleanly), a dirty worktree is force-removed after the warning, and a failed remote delete leaves the completed local - cleanup in place and reports it. The button is offered **only for a linked worktree on a non-default branch** + cleanup in place and reports it. Each git step is **retried on transient failures** — a worktree file still + held by an editor or antivirus scan, a racing git ref `.lock`, or a network blip deleting the branch on + `origin` — a few times with backoff (narrated in the flight log) before it counts; permanent refusals + (`use --force`, `remote ref does not exist`) still fail fast. The button is offered **only for a linked worktree on a non-default branch** — the clone's main working tree can't be worktree-removed, and `main`/`master` are deliberately never offered. Nothing is deleted unless you confirm; Cancel, Enter, and Esc all back out safely, and the destructive button is out of the keyboard tab order so it can't be triggered by a stray keypress. diff --git a/Docs/Features.md b/Docs/Features.md index 2743cb5..4f316a7 100644 --- a/Docs/Features.md +++ b/Docs/Features.md @@ -128,7 +128,12 @@ tidying up a branch you're finished with, in one step: **deletes the local branch**, and — when it exists — **deletes the branch on `origin`**. The git steps run from the clone's **main working tree**, so the worktree is dropped cleanly; a dirty worktree is force-removed after the warning. -- If the remote delete fails (say you're offline), the completed **local** cleanup stays done and the +- Each git step is **retried on transient failures** so a fleeting hiccup doesn't leave a half-tidied branch: + a worktree file still held open by an editor or antivirus scan (common on Windows), a git ref/index `.lock` + left by a racing git process, or a network blip while deleting the branch on `origin`. Fido retries a few + times with a short, backing-off wait — each attempt narrated in the flight log — while **permanent** refusals + (`use --force to delete`, `remote ref does not exist`, "not fully merged") still fail fast on the first try. +- If the remote delete fails for good (say you're offline), the completed **local** cleanup stays done and the failure is reported in the flight log rather than rolled back. The button is offered **only for a linked worktree on a non-default branch** — a clone's **main working diff --git a/src/Fido.csproj b/src/Fido.csproj index ad90afe..debf071 100644 --- a/src/Fido.csproj +++ b/src/Fido.csproj @@ -39,6 +39,10 @@ + + diff --git a/src/Services/GitRetry.cs b/src/Services/GitRetry.cs new file mode 100644 index 0000000..df039c8 --- /dev/null +++ b/src/Services/GitRetry.cs @@ -0,0 +1,164 @@ +using System; +using System.Threading; +using Polly; +using Polly.Retry; + +namespace Fido.Services; + +/// Tuning for 's deletion pipeline. The defaults suit an interactive desktop +/// cleanup — a few quick, backing-off retries — and tests dial the delay down to zero to stay fast. +public sealed record GitRetryOptions +{ + /// How many times to re-run a transiently-failing command (on top of the first attempt). + public int MaxRetryAttempts { get; init; } = 3; + + /// Base wait before the first retry; later waits grow per . + public TimeSpan Delay { get; init; } = TimeSpan.FromMilliseconds(250); + + /// Growth of the wait between retries. Exponential by default (0.25s, 0.5s, 1s …). + public DelayBackoffType BackoffType { get; init; } = DelayBackoffType.Exponential; + + /// Spread the waits a little so parallel callers don't all retry in lockstep. + public bool UseJitter { get; init; } = true; + + /// The profile used for worktree/branch deletion in production. + public static GitRetryOptions Default { get; } = new(); +} + +/// One retry about to happen, surfaced so the caller can narrate the wait in the flight log. +/// Human label for the command being retried (e.g. "worktree remove"). +/// Zero-based index of the attempt that just failed (0 = the first try). +/// How long the pipeline will wait before the next attempt. +/// The failed result that triggered the retry, if one was produced. +public readonly record struct GitRetryAttempt(string Operation, int AttemptNumber, TimeSpan RetryDelay, ProcessResult? Failure); + +/// +/// A Polly retry pipeline for the transient failures git's deletion commands hit — a worktree file +/// still held by an editor or antivirus scan (Windows especially), a git ref/index .lock left by a +/// racing git process, or a network blip while deleting the branch on origin. Only failures that look +/// transient (see ) are retried; a permanent failure ("use --force", "remote ref +/// does not exist") is returned on the first attempt so the caller's own handling — and the tests — stay fast. +/// +public static class GitRetry +{ + /// Carries the operation label into OnRetry so the retry narration can name the command. + private static readonly ResiliencePropertyKey OperationKey = new("fido.git.operation"); + + /// + /// Builds a reusable pipeline that retries transient git failures per . Each retry + /// invokes (if given) so a UI can narrate the wait. The predicate never handles + /// exceptions, so cancellation propagates immediately rather than being retried. + /// + public static ResiliencePipeline BuildPipeline(GitRetryOptions options, Action? onRetry = null) + { + return new ResiliencePipelineBuilder() + .AddRetry(new RetryStrategyOptions + { + ShouldHandle = static args => + { + var result = args.Outcome.Result; + return new ValueTask(result is not null && !result.Success && IsTransient(result)); + }, + MaxRetryAttempts = options.MaxRetryAttempts, + Delay = options.Delay, + BackoffType = options.BackoffType, + UseJitter = options.UseJitter, + OnRetry = args => + { + if (onRetry is not null) + { + var operation = args.Context.Properties.GetValue(OperationKey, "git"); + onRetry(new GitRetryAttempt(operation, args.AttemptNumber, args.RetryDelay, args.Outcome.Result)); + } + return default; + }, + }) + .Build(); + } + + /// + /// Runs through , tagging the flow with + /// so retries can be narrated, and threading the caller's cancellation token + /// down to each attempt. + /// + public static async Task ExecuteAsync( + ResiliencePipeline pipeline, + string operation, + Func> action, + CancellationToken ct = default) + { + var context = ResilienceContextPool.Shared.Get(ct); + context.Properties.Set(OperationKey, operation); + try + { + return await pipeline.ExecuteAsync( + static (ctx, state) => new ValueTask(state(ctx.CancellationToken)), + context, + action); + } + finally + { + ResilienceContextPool.Shared.Return(context); + } + } + + /// + /// True when a failed git result looks worth retrying — a filesystem/lock hiccup or a network blip — + /// rather than a permanent refusal. Matches a curated set of markers in the command's stderr/stdout, + /// case-insensitively. Always false for a successful result. + /// + public static bool IsTransient(ProcessResult result) + { + if (result.Success) return false; + + var text = result.StdErr + "\n" + result.StdOut; + foreach (var marker in TransientMarkers) + if (text.Contains(marker, StringComparison.OrdinalIgnoreCase)) + return true; + return false; + } + + /// + /// Substrings that flag a retryable failure. Two families: + /// + /// Filesystem / lock contention — a worktree file still held open (an editor, an antivirus scan) or a + /// git ref/index .lock left by a racing git process. On Windows a locked unlink surfaces as + /// "Permission denied" / "Access is denied" / "being used by another process"; these clear on their own, + /// so a brief retry usually wins. (An SSH auth "Permission denied (publickey)" would also match, but that's + /// a non-fatal remote-delete whose retry merely costs a second before the same report — an acceptable trade + /// for catching the far more common file-lock case.) + /// Network — deleting the branch on origin over a flaky connection. + /// + /// Deliberately narrow so permanent refusals ("use --force to delete", "remote ref does not exist", + /// "not fully merged") are not matched and fail fast on the first attempt. + /// + private static readonly string[] TransientMarkers = + [ + // Filesystem / lock contention. + "being used by another process", + "access is denied", + "permission denied", + "resource temporarily unavailable", + ".lock", + "cannot lock ref", + "unable to create", + "could not lock", + // Network. + "could not resolve host", + "couldn't resolve host", + "connection timed out", + "connection reset", + "connection refused", + "failed to connect", + "unable to access", + "could not read from remote repository", + "the remote end hung up unexpectedly", + "rpc failed", + "early eof", + "operation timed out", + "temporary failure in name resolution", + "network is unreachable", + "no route to host", + "ssh: connect to host", + ]; +} diff --git a/src/Services/GitService.cs b/src/Services/GitService.cs index df96909..600efbd 100644 --- a/src/Services/GitService.cs +++ b/src/Services/GitService.cs @@ -9,9 +9,21 @@ public sealed class GitService { private const string RefsHeads = "refs/heads/"; - private static Task Git(string dir, CancellationToken ct, params string[] args) + /// Runs a git command in and returns its captured result. The + /// default shells out to the real git CLI; tests inject a fake to script output (e.g. a transient + /// failure that then clears) without needing to provoke one from a real repository. + public delegate Task GitCommandRunner(string workingDir, IReadOnlyList args, CancellationToken ct); + + private readonly GitCommandRunner _run; + + public GitService(GitCommandRunner? run = null) => _run = run ?? new GitCommandRunner(DefaultRun); + + private static Task DefaultRun(string dir, IReadOnlyList args, CancellationToken ct) => ProcessRunner.RunAsync("git", args, dir, ct); + private Task Git(string dir, CancellationToken ct, params string[] args) + => _run(dir, args, ct); + public async Task IsInsideWorkTreeAsync(string dir, CancellationToken ct = default) { var r = await Git(dir, ct, "rev-parse", "--is-inside-work-tree"); diff --git a/src/Services/OpenerService.cs b/src/Services/OpenerService.cs index 6f35b60..81b0c7c 100644 --- a/src/Services/OpenerService.cs +++ b/src/Services/OpenerService.cs @@ -2,6 +2,7 @@ using System.IO; using System.Threading; using Fido.Models; +using Polly; namespace Fido.Services; @@ -37,14 +38,24 @@ public sealed class OpenerService private readonly Action _log; private readonly Action _liveLog; + /// Retries the transient failures the worktree/branch deletion commands hit (locked files, ref + /// .lock races, network blips), narrating each retry into the flight log. See . + private readonly ResiliencePipeline _deletionRetry; + public OpenerService(GitService git, SolutionFinder finder, WorkingTreeFinder workingTreeFinder, - Action? log = null, Action? liveLog = null) + Action? log = null, Action? liveLog = null, GitRetryOptions? deletionRetry = null) { _git = git; _finder = finder; _workingTreeFinder = workingTreeFinder; _log = log ?? (_ => { }); _liveLog = liveLog ?? (_ => { }); + + var retryOptions = deletionRetry ?? GitRetryOptions.Default; + _deletionRetry = GitRetry.BuildPipeline(retryOptions, attempt => + _log($"[!] {attempt.Operation} failed (transient) — retrying " + + $"{attempt.AttemptNumber + 1}/{retryOptions.MaxRetryAttempts} in " + + $"{attempt.RetryDelay.TotalSeconds:0.#}s: {attempt.Failure?.Message}")); } /// @@ -401,10 +412,13 @@ public Task IsLinkedWorktreeAsync(string folder, CancellationToken ct = de /// Carries out a limited to the targets the user ticked in /// : removes the worktree (forcing when it's dirty), deletes the local branch, and /// deletes the branch on origin — each only when selected (and the origin branch only when it exists). - /// Runs from the clone's main tree. A failed worktree removal or local-branch delete throws (nothing has - /// been lost yet, or the local cleanup couldn't proceed); a failed remote delete is logged and - /// reflected in the returned outcome rather than throwing, because any local cleanup is already done and - /// re-running wouldn't undo it. + /// Runs from the clone's main tree. Each git step is wrapped in , so a + /// transient failure — a worktree file still locked by an editor, a racing ref .lock, a + /// network blip on the origin delete — is retried a few times (narrated in the log) before it counts; a + /// permanent failure fails on the first attempt. A failed worktree removal or local-branch delete throws + /// (nothing has been lost yet, or the local cleanup couldn't proceed); a failed remote delete is + /// logged and reflected in the returned outcome rather than throwing, because any local cleanup is already + /// done and re-running wouldn't undo it. /// public async Task DeleteWorktreeAsync( WorktreeDeletion plan, WorktreeDeletionChoice choice, CancellationToken ct = default) @@ -415,7 +429,8 @@ public async Task DeleteWorktreeAsync( if (choice.Worktree) { _log($"Removing worktree at {plan.WorktreePath}…"); - var remove = await _git.WorktreeRemoveAsync(dir, plan.WorktreePath, force: plan.HasOutstandingChanges, ct); + var remove = await GitRetry.ExecuteAsync(_deletionRetry, "worktree remove", + token => _git.WorktreeRemoveAsync(dir, plan.WorktreePath, force: plan.HasOutstandingChanges, token), ct); if (!remove.Success) throw new InvalidOperationException($"git worktree remove failed: {remove.Message}"); _log("Worktree removed."); @@ -425,7 +440,8 @@ public async Task DeleteWorktreeAsync( if (choice.LocalBranch) { _log($"Deleting local branch '{plan.Branch}'…"); - var branchResult = await _git.DeleteLocalBranchAsync(dir, plan.Branch, ct); + var branchResult = await GitRetry.ExecuteAsync(_deletionRetry, "local branch delete", + token => _git.DeleteLocalBranchAsync(dir, plan.Branch, token), ct); if (!branchResult.Success) throw new InvalidOperationException($"git branch -D failed: {branchResult.Message}"); _log($"Local branch '{plan.Branch}' deleted."); @@ -435,7 +451,8 @@ public async Task DeleteWorktreeAsync( if (choice.RemoteBranch && plan.RemoteBranchExists) { _log($"Deleting remote branch origin/{plan.Branch}…"); - var remoteResult = await _git.DeleteRemoteBranchAsync(dir, plan.Branch, ct); + var remoteResult = await GitRetry.ExecuteAsync(_deletionRetry, "remote branch delete", + token => _git.DeleteRemoteBranchAsync(dir, plan.Branch, token), ct); if (remoteResult.Success) { _log($"Remote branch origin/{plan.Branch} deleted."); diff --git a/tests/Fido.Tests/Services/GitRetryTests.cs b/tests/Fido.Tests/Services/GitRetryTests.cs new file mode 100644 index 0000000..092d5cf --- /dev/null +++ b/tests/Fido.Tests/Services/GitRetryTests.cs @@ -0,0 +1,128 @@ +using Fido.Services; +using Polly; + +namespace Fido.Tests.Services; + +/// +/// The retry policy behind reliable worktree/branch deletion: which git failures count as transient (worth a +/// retry) versus permanent (fail fast), and that the pipeline actually re-runs the flaky ones and gives up +/// after the configured budget. All in-memory — no git, no waiting (the delay is dialled to zero). +/// +public class GitRetryTests +{ + /// Zero-delay so the retry loop runs instantly under test. + private static readonly GitRetryOptions Fast = new() + { + MaxRetryAttempts = 3, + Delay = TimeSpan.Zero, + UseJitter = false, + BackoffType = DelayBackoffType.Constant, + }; + + private static ProcessResult Fail(string stderr) => new(1, "", stderr); + private static ProcessResult Ok() => new(0, "", ""); + + // --- IsTransient: the transient/permanent classification ----------------------------- + + [Test] + [Arguments("fatal: could not remove worktree: 'a.txt': being used by another process")] + [Arguments("error: unable to unlink old 'src/x': Permission denied")] + [Arguments("error: unable to delete 'x': Access is denied")] + [Arguments("fatal: Unable to create '/repo/.git/worktrees/x/HEAD.lock': File exists")] + [Arguments("error: cannot lock ref 'refs/heads/feature/x': is at 0000 but expected 1111")] + [Arguments("fatal: unable to access 'https://github.com/o/r.git/': Could not resolve host: github.com")] + [Arguments("ssh: connect to host github.com port 22: Connection timed out")] + [Arguments("error: RPC failed; curl 56 Recv failure: Connection reset by peer")] + [Arguments("fatal: the remote end hung up unexpectedly")] + public async Task Classifies_lock_and_network_failures_as_transient(string stderr) + => await Assert.That(GitRetry.IsTransient(Fail(stderr))).IsTrue(); + + [Test] + [Arguments("fatal: 'feature/x' contains modified or untracked files, use --force to delete it")] + [Arguments("error: unable to delete 'feature/x': remote ref does not exist\nerror: failed to push some refs to 'origin'")] + [Arguments("error: The branch 'feature/x' is not fully merged.")] + [Arguments("error: branch 'feature/x' not found.")] + public async Task Classifies_permanent_refusals_as_not_transient(string stderr) + => await Assert.That(GitRetry.IsTransient(Fail(stderr))).IsFalse(); + + [Test] + public async Task A_successful_result_is_never_transient() + => await Assert.That(GitRetry.IsTransient(Ok())).IsFalse(); + + // --- Pipeline behaviour --------------------------------------------------------------- + + [Test] + public async Task Retries_a_transient_failure_until_it_succeeds() + { + var attempts = new List(); + var pipeline = GitRetry.BuildPipeline(Fast, attempts.Add); + + var calls = 0; + var result = await GitRetry.ExecuteAsync(pipeline, "worktree remove", _ => + { + calls++; + return Task.FromResult(calls < 3 ? Fail("being used by another process") : Ok()); + }); + + await Assert.That(result.Success).IsTrue(); + await Assert.That(calls).IsEqualTo(3); // failed twice, then succeeded + + // Two retries, each carrying the operation label and a 0-based index of the attempt that failed. + await Assert.That(attempts.Count).IsEqualTo(2); + await Assert.That(attempts[0].Operation).IsEqualTo("worktree remove"); + await Assert.That(attempts[0].AttemptNumber).IsEqualTo(0); + await Assert.That(attempts[1].AttemptNumber).IsEqualTo(1); + } + + [Test] + public async Task Does_not_retry_a_permanent_failure() + { + var attempts = new List(); + var pipeline = GitRetry.BuildPipeline(Fast, attempts.Add); + + var calls = 0; + var result = await GitRetry.ExecuteAsync(pipeline, "worktree remove", _ => + { + calls++; + return Task.FromResult(Fail("use --force to delete it")); + }); + + await Assert.That(result.Success).IsFalse(); + await Assert.That(calls).IsEqualTo(1); // one attempt, no retries + await Assert.That(attempts.Count).IsEqualTo(0); + } + + [Test] + public async Task Gives_up_after_the_retry_budget_on_a_persistent_transient_failure() + { + var pipeline = GitRetry.BuildPipeline(Fast); + + var calls = 0; + var result = await GitRetry.ExecuteAsync(pipeline, "remote branch delete", _ => + { + calls++; + return Task.FromResult(Fail("fatal: unable to access 'https://o/r': Could not resolve host: o")); + }); + + await Assert.That(result.Success).IsFalse(); // the final failure is returned, not thrown + await Assert.That(calls).IsEqualTo(4); // first try + 3 retries + } + + [Test] + public async Task Runs_a_first_time_success_exactly_once() + { + var attempts = new List(); + var pipeline = GitRetry.BuildPipeline(Fast, attempts.Add); + + var calls = 0; + var result = await GitRetry.ExecuteAsync(pipeline, "local branch delete", _ => + { + calls++; + return Task.FromResult(Ok()); + }); + + await Assert.That(result.Success).IsTrue(); + await Assert.That(calls).IsEqualTo(1); + await Assert.That(attempts.Count).IsEqualTo(0); + } +} diff --git a/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs b/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs new file mode 100644 index 0000000..9c10098 --- /dev/null +++ b/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs @@ -0,0 +1,123 @@ +using Fido.Models; +using Fido.Services; +using Polly; + +namespace Fido.Tests.Services; + +/// +/// End-to-end wiring of the deletion retry: re-runs a +/// git delete step that fails transiently and gives up on one that fails permanently. Git is faked through +/// so the transient failures are scripted rather than provoked, +/// and the retry delay is zero so the tests don't wait. +/// +public class OpenerDeletionRetryTests +{ + /// Zero-delay retry so the loop runs instantly. + private static readonly GitRetryOptions Fast = new() + { + MaxRetryAttempts = 3, + Delay = TimeSpan.Zero, + UseJitter = false, + BackoffType = DelayBackoffType.Constant, + }; + + private static WorktreeDeletion Plan() => new( + MainWorktreePath: "/repo", + WorktreePath: "/repo.worktrees/feature-x", + Branch: "feature/x", + RemoteBranchExists: true, + OutstandingChanges: Array.Empty(), + OrphanedCommits: 0); + + private static OpenerService Opener(GitService.GitCommandRunner runner, List? log = null) => + new(new GitService(runner), new SolutionFinder(), new WorkingTreeFinder(), + log: log is null ? null : log.Add, deletionRetry: Fast); + + private static bool Matches(IReadOnlyList args, params string[] prefix) + { + if (args.Count < prefix.Length) return false; + for (var i = 0; i < prefix.Length; i++) + if (args[i] != prefix[i]) return false; + return true; + } + + [Test] + public async Task Retries_a_transiently_locked_worktree_removal_then_completes_the_delete() + { + var removeCalls = 0; + var log = new List(); + GitService.GitCommandRunner runner = (_, args, _) => + { + if (Matches(args, "worktree", "remove")) + { + removeCalls++; + // Windows: a file in the worktree is still held open by an editor — clears after a moment. + return Task.FromResult(removeCalls < 3 + ? new ProcessResult(1, "", $"fatal: failed to delete '{args[^1]}': being used by another process") + : new ProcessResult(0, "", "")); + } + return Task.FromResult(new ProcessResult(0, "", "")); // branch -D and push --delete succeed + }; + + var outcome = await Opener(runner, log).DeleteWorktreeAsync(Plan(), WorktreeDeletionChoice.All); + + await Assert.That(removeCalls).IsEqualTo(3); // failed twice, third try stuck + await Assert.That(outcome.WorktreeRemoved).IsTrue(); + await Assert.That(outcome.LocalBranchDeleted).IsTrue(); + await Assert.That(outcome.RemoteBranchDeleted).IsTrue(); + await Assert.That(outcome.RemoteDeleteFailed).IsFalse(); + await Assert.That(log.Any(l => l.Contains("worktree remove failed (transient)"))).IsTrue(); + } + + [Test] + public async Task Retries_a_transient_remote_delete_then_reports_the_remote_gone() + { + var pushCalls = 0; + GitService.GitCommandRunner runner = (_, args, _) => + { + if (Matches(args, "push")) + { + pushCalls++; + return Task.FromResult(pushCalls < 2 + ? new ProcessResult(128, "", "fatal: unable to access 'https://origin/r.git/': Could not resolve host: origin") + : new ProcessResult(0, "", "")); + } + return Task.FromResult(new ProcessResult(0, "", "")); // worktree remove and branch -D succeed + }; + + var outcome = await Opener(runner).DeleteWorktreeAsync(Plan(), WorktreeDeletionChoice.All); + + await Assert.That(pushCalls).IsEqualTo(2); // failed once (host blip), retried, then gone + await Assert.That(outcome.RemoteBranchDeleted).IsTrue(); + await Assert.That(outcome.RemoteDeleteFailed).IsFalse(); + } + + [Test] + public async Task Does_not_retry_a_permanent_worktree_removal_failure() + { + var removeCalls = 0; + GitService.GitCommandRunner runner = (_, args, _) => + { + if (Matches(args, "worktree", "remove")) + { + removeCalls++; + return Task.FromResult(new ProcessResult(128, "", + "fatal: 'feature/x' contains modified or untracked files, use --force to delete it")); + } + return Task.FromResult(new ProcessResult(0, "", "")); + }; + + InvalidOperationException? thrown = null; + try + { + await Opener(runner).DeleteWorktreeAsync(Plan(), WorktreeDeletionChoice.All); + } + catch (InvalidOperationException ex) + { + thrown = ex; // a permanent worktree-remove failure still throws — nothing has been lost yet + } + + await Assert.That(thrown).IsNotNull(); + await Assert.That(removeCalls).IsEqualTo(1); // one attempt, no wasted retries + } +} From fba06e1d27cc34f79ea3d483faefb72d68e78f34 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 15:44:33 +0000 Subject: [PATCH 2/4] Tighten transient-failure classification for deletion retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up from an adversarial review of the retry classifier: - Fix an over-broad marker: a bare ".lock" substring also matched a *permanent* "use --force" failure whenever it echoed a worktree path containing ".lock" (e.g. a branch named fix.lockfile-bug), needlessly retrying it. Match git's lock-creation phrasing instead ("cannot lock ref", "unable to create … .lock", "another git process seems to be running"), which still covers real lock contention. Added a regression test. - Broaden coverage for the cases the feature targets: filesystem "directory not empty" / "device or resource busy", and transient network HTTP-5xx, TLS-handshake, and sideband-disconnect errors. - Document, in the marker list, that permanent remote auth failures (HTTP 403 / SSH publickey) are knowingly retried — bounded and non-fatal — and, in DeleteWorktreeAsync, the rare non-destructive case where a drop after a successful origin delete is re-reported as failed. All 188 tests pass; clean Native AOT publish retained. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01KfYxaFc4xQHTt75dhdQoWx --- src/Services/GitRetry.cs | 31 ++++++++++++++++------ src/Services/OpenerService.cs | 6 +++++ tests/Fido.Tests/Services/GitRetryTests.cs | 8 ++++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/Services/GitRetry.cs b/src/Services/GitRetry.cs index df039c8..1dbbb60 100644 --- a/src/Services/GitRetry.cs +++ b/src/Services/GitRetry.cs @@ -121,16 +121,23 @@ public static bool IsTransient(ProcessResult result) /// /// Substrings that flag a retryable failure. Two families: /// - /// Filesystem / lock contention — a worktree file still held open (an editor, an antivirus scan) or a - /// git ref/index .lock left by a racing git process. On Windows a locked unlink surfaces as - /// "Permission denied" / "Access is denied" / "being used by another process"; these clear on their own, - /// so a brief retry usually wins. (An SSH auth "Permission denied (publickey)" would also match, but that's - /// a non-fatal remote-delete whose retry merely costs a second before the same report — an acceptable trade - /// for catching the far more common file-lock case.) + /// Filesystem / lock contention — a worktree file still held open (an editor, an antivirus scan), a + /// file the OS still reports busy, or a git ref/index .lock left by a racing git process. On Windows + /// a locked unlink surfaces as "Permission denied" / "Access is denied" / "being used by another process"; + /// these clear on their own, so a brief retry usually wins. /// Network — deleting the branch on origin over a flaky connection. /// /// Deliberately narrow so permanent refusals ("use --force to delete", "remote ref does not exist", - /// "not fully merged") are not matched and fail fast on the first attempt. + /// "not fully merged") are not matched and fail fast on the first attempt. In particular, lock + /// contention is matched by git's lock-creation phrasing ("cannot lock ref", "unable to create … + /// … .lock", "another git process seems to be running") rather than a bare ".lock" — which would + /// also match a permanent failure that merely echoes a worktree path containing ".lock" (a branch + /// like fix.lockfile-bug). + /// Known over-matches, all bounded and non-fatal: a permanent remote auth failure — HTTP 403 + /// ("unable to access" / "could not read from remote repository") or SSH "Permission denied (publickey)" — + /// is retried the full budget before the same report. Accepted: remote-delete failures don't roll back the + /// local cleanup, and the cost is a couple of seconds against catching the far more common file-lock and + /// transient-network cases. /// private static readonly string[] TransientMarkers = [ @@ -139,10 +146,13 @@ public static bool IsTransient(ProcessResult result) "access is denied", "permission denied", "resource temporarily unavailable", - ".lock", + "device or resource busy", + "directory not empty", "cannot lock ref", + "unable to lock", "unable to create", "could not lock", + "another git process seems to be running", // Network. "could not resolve host", "couldn't resolve host", @@ -155,10 +165,15 @@ public static bool IsTransient(ProcessResult result) "the remote end hung up unexpectedly", "rpc failed", "early eof", + "unexpected disconnect while reading sideband packet", "operation timed out", "temporary failure in name resolution", "network is unreachable", "no route to host", "ssh: connect to host", + "the requested url returned error: 5", + "gnutls_handshake", + "openssl ssl_read", + "schannel: failed", ]; } diff --git a/src/Services/OpenerService.cs b/src/Services/OpenerService.cs index 81b0c7c..073d3e6 100644 --- a/src/Services/OpenerService.cs +++ b/src/Services/OpenerService.cs @@ -451,6 +451,12 @@ public async Task DeleteWorktreeAsync( if (choice.RemoteBranch && plan.RemoteBranchExists) { _log($"Deleting remote branch origin/{plan.Branch}…"); + // Retrying the push is safe — deleting an already-gone branch is a no-op in effect. One rare, + // non-destructive wrinkle: if a transient drop happens *after* origin deleted the ref but before + // git reads the ack, the retry sees "remote ref does not exist" (permanent) and reports failure + // though the branch is in fact gone. We don't infer success from that message — on a first attempt + // it legitimately means the ref was already gone, which callers surface as a NO-GO — so we accept + // the occasional misleading report over guessing. var remoteResult = await GitRetry.ExecuteAsync(_deletionRetry, "remote branch delete", token => _git.DeleteRemoteBranchAsync(dir, plan.Branch, token), ct); if (remoteResult.Success) diff --git a/tests/Fido.Tests/Services/GitRetryTests.cs b/tests/Fido.Tests/Services/GitRetryTests.cs index 092d5cf..5aa550d 100644 --- a/tests/Fido.Tests/Services/GitRetryTests.cs +++ b/tests/Fido.Tests/Services/GitRetryTests.cs @@ -28,17 +28,25 @@ public class GitRetryTests [Arguments("fatal: could not remove worktree: 'a.txt': being used by another process")] [Arguments("error: unable to unlink old 'src/x': Permission denied")] [Arguments("error: unable to delete 'x': Access is denied")] + [Arguments("fatal: could not remove worktree directory '/repo.worktrees/feature-x': Directory not empty")] + [Arguments("error: unable to unlink old 'src/x': Device or resource busy")] [Arguments("fatal: Unable to create '/repo/.git/worktrees/x/HEAD.lock': File exists")] + [Arguments("fatal: Unable to create '/repo/.git/index.lock': File exists.\n\nAnother git process seems to be running in this repository")] [Arguments("error: cannot lock ref 'refs/heads/feature/x': is at 0000 but expected 1111")] [Arguments("fatal: unable to access 'https://github.com/o/r.git/': Could not resolve host: github.com")] + [Arguments("fatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 503")] [Arguments("ssh: connect to host github.com port 22: Connection timed out")] [Arguments("error: RPC failed; curl 56 Recv failure: Connection reset by peer")] [Arguments("fatal: the remote end hung up unexpectedly")] + [Arguments("error: RPC failed; curl 92 HTTP/2 stream 5 was reset\nfatal: unexpected disconnect while reading sideband packet")] public async Task Classifies_lock_and_network_failures_as_transient(string stderr) => await Assert.That(GitRetry.IsTransient(Fail(stderr))).IsTrue(); [Test] [Arguments("fatal: 'feature/x' contains modified or untracked files, use --force to delete it")] + // The permanent "use --force" failure echoes the worktree path; a branch whose name contains ".lock" + // must NOT be misread as lock contention (regression guard for the over-broad bare ".lock" marker). + [Arguments("fatal: '/repo.worktrees/fix.lockfile-bug' contains modified or untracked files, use --force to delete it")] [Arguments("error: unable to delete 'feature/x': remote ref does not exist\nerror: failed to push some refs to 'origin'")] [Arguments("error: The branch 'feature/x' is not fully merged.")] [Arguments("error: branch 'feature/x' not found.")] From 712214626ea94dc16baa8e84046df6f287548ffe Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 16:23:18 +0000 Subject: [PATCH 3/4] Handle long worktree paths and offer a permanent folder delete Deleting a worktree could fail with "filename too long" on Windows when the tree crossed the 260-char MAX_PATH limit (deep node_modules, generated output). Two-part fix: - Run git's worktree add/remove with `-c core.longpaths=true` so git's own file operations use the Windows extended-length API. - When git still can't remove the folder, DeleteWorktreeAsync now throws a typed WorktreeRemovalException and the UI offers a fallback: a permanent, recursive folder delete that bypasses the Recycle Bin (using a \\?\ extended-length path so it isn't stopped by the same limit), followed by `git worktree prune` to clear the dangling registration and then the ticked branch deletions. It's an explicit, clearly-labelled confirmation (new ForceDeleteDialog); backing out is a NO-GO and leaves everything in place. Tests: git passes the long-paths flag; ForceDeleteWorktreeAsync deletes the folder + prunes + deletes branches (incl. keeping an unticked branch); and the full window flow through the offer, accepted and declined, driven against real git via an injected worktree-remove failure. All 193 tests pass; clean Native AOT publish retained. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01KfYxaFc4xQHTt75dhdQoWx --- CHANGELOG.md | 7 +- Docs/Features.md | 8 ++ src/Models/WorktreeForceDelete.cs | 10 +++ src/Services/AvaloniaDialogService.cs | 3 + src/Services/GitService.cs | 23 +++-- src/Services/IDialogService.cs | 7 ++ src/Services/OpenerService.cs | 87 ++++++++++++++++-- src/Services/WorktreeRemovalException.cs | 18 ++++ src/Views/ForceDeleteDialog.axaml | 49 ++++++++++ src/Views/ForceDeleteDialog.axaml.cs | 29 ++++++ src/Views/MainWindow.axaml.cs | 23 ++++- tests/Fido.Tests/E2E/DeleteWorktreeTests.cs | 89 +++++++++++++++++++ .../Infrastructure/FakeDialogService.cs | 13 +++ .../Infrastructure/TestRepoWorld.cs | 4 +- tests/Fido.Tests/Services/GitServiceTests.cs | 25 ++++++ .../Services/OpenerDeletionRetryTests.cs | 23 +++-- .../Services/OpenerForceDeleteTests.cs | 77 ++++++++++++++++ 17 files changed, 469 insertions(+), 26 deletions(-) create mode 100644 src/Models/WorktreeForceDelete.cs create mode 100644 src/Services/WorktreeRemovalException.cs create mode 100644 src/Views/ForceDeleteDialog.axaml create mode 100644 src/Views/ForceDeleteDialog.axaml.cs create mode 100644 tests/Fido.Tests/Services/OpenerForceDeleteTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index b19a638..55e7c06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 cleanup in place and reports it. Each git step is **retried on transient failures** — a worktree file still held by an editor or antivirus scan, a racing git ref `.lock`, or a network blip deleting the branch on `origin` — a few times with backoff (narrated in the flight log) before it counts; permanent refusals - (`use --force`, `remote ref does not exist`) still fail fast. The button is offered **only for a linked worktree on a non-default branch** + (`use --force`, `remote ref does not exist`) still fail fast. Git's worktree commands run with **long-path + support** (`core.longpaths`) so a deep tree that crosses Windows' 260-character `MAX_PATH` limit (deep + `node_modules`, generated output) can still be created and removed. If git **still** can't delete the folder, + Fido **offers to delete it permanently from disk** — a recursive removal that **bypasses the Recycle Bin** + (using an extended-length path so it isn't stopped by the same limit) — and then prunes git's now-dangling + worktree registration so the branch can be deleted too. The button is offered **only for a linked worktree on a non-default branch** — the clone's main working tree can't be worktree-removed, and `main`/`master` are deliberately never offered. Nothing is deleted unless you confirm; Cancel, Enter, and Esc all back out safely, and the destructive button is out of the keyboard tab order so it can't be triggered by a stray keypress. diff --git a/Docs/Features.md b/Docs/Features.md index 4f316a7..9844cce 100644 --- a/Docs/Features.md +++ b/Docs/Features.md @@ -133,6 +133,14 @@ tidying up a branch you're finished with, in one step: left by a racing git process, or a network blip while deleting the branch on `origin`. Fido retries a few times with a short, backing-off wait — each attempt narrated in the flight log — while **permanent** refusals (`use --force to delete`, `remote ref does not exist`, "not fully merged") still fail fast on the first try. +- **Long paths & a force-delete fallback.** Git's worktree commands run with **long-path support** + (`core.longpaths`), so a worktree whose files cross Windows' **260-character `MAX_PATH`** limit — deep + `node_modules`, generated output — can still be removed. If git **still** can't delete the folder (a path too + long even for that), Fido **offers to delete it straight from disk**: a recursive removal that **bypasses the + Recycle Bin** and uses an extended-length (`\\?\`) path so it isn't defeated by the same limit. Once the + folder is gone Fido **prunes** git's dangling worktree registration and carries on with the branch deletions. + It's an explicit, clearly-labelled confirmation — nothing is force-deleted unless you choose it, and backing + out leaves everything in place. - If the remote delete fails for good (say you're offline), the completed **local** cleanup stays done and the failure is reported in the flight log rather than rolled back. diff --git a/src/Models/WorktreeForceDelete.cs b/src/Models/WorktreeForceDelete.cs new file mode 100644 index 0000000..baee134 --- /dev/null +++ b/src/Models/WorktreeForceDelete.cs @@ -0,0 +1,10 @@ +namespace Fido.Models; + +/// +/// A prompt to permanently delete a worktree folder that git worktree remove couldn't — most often +/// because a path inside it is too long for the OS. The delete is a direct, recursive removal from disk: it +/// bypasses the Recycle Bin and can't be undone. +/// +/// Absolute path of the worktree folder to delete. +/// The git failure that prompted the offer, shown so the user knows why it's needed. +public sealed record WorktreeForceDelete(string WorktreePath, string Reason); diff --git a/src/Services/AvaloniaDialogService.cs b/src/Services/AvaloniaDialogService.cs index cdcca8a..ae9eb94 100644 --- a/src/Services/AvaloniaDialogService.cs +++ b/src/Services/AvaloniaDialogService.cs @@ -18,6 +18,9 @@ public sealed class AvaloniaDialogService : IDialogService public Task ConfirmDeleteWorktreeAsync(WorktreeDeletion plan) => new DeleteWorktreeDialog(plan).ShowDialog(_owner); + public Task ConfirmForceDeleteWorktreeFolderAsync(WorktreeForceDelete request) + => new ForceDeleteDialog(request).ShowDialog(_owner); + public Task ShowDecisionAsync(RepositoryInfo repo, string branch, MainContext context) => new DecisionDialog(repo, branch, context).ShowDialog(_owner); diff --git a/src/Services/GitService.cs b/src/Services/GitService.cs index 600efbd..71ce3dc 100644 --- a/src/Services/GitService.cs +++ b/src/Services/GitService.cs @@ -173,13 +173,18 @@ public Task SwitchNewFromAsync(string dir, string branch, string? // --- Worktree creation -------------------------------------------------------------- + // `-c core.longpaths=true` lets git's own file operations use the Windows extended-length API, so a + // worktree whose checked-out files cross the 260-char MAX_PATH limit (deep node_modules, generated output) + // can still be created and removed. Harmless on other platforms and when a shorter path is used. + private const string LongPaths = "core.longpaths=true"; + public Task WorktreeAddExistingAsync(string dir, string path, string branch, CancellationToken ct = default) - => Git(dir, ct, "worktree", "add", path, branch); + => Git(dir, ct, "-c", LongPaths, "worktree", "add", path, branch); public Task WorktreeAddNewAsync(string dir, string path, string branch, string? startPoint, CancellationToken ct = default) => startPoint is null - ? Git(dir, ct, "worktree", "add", "-b", branch, path) - : Git(dir, ct, "worktree", "add", "-b", branch, path, startPoint); + ? Git(dir, ct, "-c", LongPaths, "worktree", "add", "-b", branch, path) + : Git(dir, ct, "-c", LongPaths, "worktree", "add", "-b", branch, path, startPoint); // --- Worktree / branch deletion ----------------------------------------------------- @@ -220,8 +225,16 @@ public async Task CountOrphanedCommitsAsync(string dir, string branch, Canc /// public Task WorktreeRemoveAsync(string dir, string worktreePath, bool force, CancellationToken ct = default) => force - ? Git(dir, ct, "worktree", "remove", "--force", worktreePath) - : Git(dir, ct, "worktree", "remove", worktreePath); + ? Git(dir, ct, "-c", LongPaths, "worktree", "remove", "--force", worktreePath) + : Git(dir, ct, "-c", LongPaths, "worktree", "remove", worktreePath); + + /// + /// Drops git's registration of any worktree whose directory has gone missing (git worktree prune). + /// Used after a worktree folder is deleted out-of-band — e.g. the force-delete fallback — so the branch it + /// held is no longer considered checked out and can be deleted. + /// + public Task PruneWorktreesAsync(string dir, CancellationToken ct = default) + => Git(dir, ct, "worktree", "prune"); /// /// Force-deletes the local branch (git branch -D) — used once its worktree is gone, so the diff --git a/src/Services/IDialogService.cs b/src/Services/IDialogService.cs index 21dfd01..91466d3 100644 --- a/src/Services/IDialogService.cs +++ b/src/Services/IDialogService.cs @@ -23,6 +23,13 @@ public interface IDialogService /// Task ConfirmDeleteWorktreeAsync(WorktreeDeletion plan); + /// + /// After git worktree remove fails (typically a path too long for the OS), asks whether to + /// permanently delete the folder straight from disk — a recursive delete that bypasses the Recycle Bin. + /// Returns true to proceed, false to leave it in place. + /// + Task ConfirmForceDeleteWorktreeFolderAsync(WorktreeForceDelete request); + /// Branch-not-checked-out decision; returns the chosen action, or null if dismissed. Task ShowDecisionAsync(RepositoryInfo repo, string branch, MainContext context); diff --git a/src/Services/OpenerService.cs b/src/Services/OpenerService.cs index 073d3e6..72e1792 100644 --- a/src/Services/OpenerService.cs +++ b/src/Services/OpenerService.cs @@ -415,28 +415,65 @@ public Task IsLinkedWorktreeAsync(string folder, CancellationToken ct = de /// Runs from the clone's main tree. Each git step is wrapped in , so a /// transient failure — a worktree file still locked by an editor, a racing ref .lock, a /// network blip on the origin delete — is retried a few times (narrated in the log) before it counts; a - /// permanent failure fails on the first attempt. A failed worktree removal or local-branch delete throws - /// (nothing has been lost yet, or the local cleanup couldn't proceed); a failed remote delete is - /// logged and reflected in the returned outcome rather than throwing, because any local cleanup is already - /// done and re-running wouldn't undo it. + /// permanent failure fails on the first attempt. A failed worktree removal throws a + /// (so the caller can offer as + /// a fallback); a failed local-branch delete throws (the local cleanup couldn't proceed); a failed + /// remote delete is logged and reflected in the returned outcome rather than throwing, because any + /// local cleanup is already done and re-running wouldn't undo it. /// public async Task DeleteWorktreeAsync( WorktreeDeletion plan, WorktreeDeletionChoice choice, CancellationToken ct = default) { - var dir = plan.MainWorktreePath; - bool worktreeRemoved = false, localDeleted = false, remoteDeleted = false, remoteFailed = false; + var worktreeRemoved = false; if (choice.Worktree) { _log($"Removing worktree at {plan.WorktreePath}…"); var remove = await GitRetry.ExecuteAsync(_deletionRetry, "worktree remove", - token => _git.WorktreeRemoveAsync(dir, plan.WorktreePath, force: plan.HasOutstandingChanges, token), ct); + token => _git.WorktreeRemoveAsync(plan.MainWorktreePath, plan.WorktreePath, force: plan.HasOutstandingChanges, token), ct); if (!remove.Success) - throw new InvalidOperationException($"git worktree remove failed: {remove.Message}"); + throw new WorktreeRemovalException(plan.WorktreePath, remove.Message); _log("Worktree removed."); worktreeRemoved = true; } + return await DeleteBranchesAsync(plan, choice, worktreeRemoved, ct); + } + + /// + /// Fallback for when raised a — + /// typically a path too long for the OS. Permanently deletes the worktree folder straight from disk (a + /// recursive delete that bypasses the Recycle Bin and, on Windows, uses an extended-length path so + /// it isn't defeated by the same limit that stopped git), then prunes git's now-dangling worktree + /// registration so the branch is free to delete, and finishes the ticked branch deletions. The caller must + /// have confirmed the destructive folder delete first. + /// + public async Task ForceDeleteWorktreeAsync( + WorktreeDeletion plan, WorktreeDeletionChoice choice, CancellationToken ct = default) + { + _log($"Force-deleting worktree folder {plan.WorktreePath} (bypassing the Recycle Bin)…"); + await Task.Run(() => ForceDeleteFolder(plan.WorktreePath), ct); + _log("Worktree folder deleted; pruning git's worktree registration…"); + + var prune = await _git.PruneWorktreesAsync(plan.MainWorktreePath, ct); + if (!prune.Success) + _log($"[!] git worktree prune reported: {prune.Message}"); + + return await DeleteBranchesAsync(plan, choice, worktreeRemoved: true, ct); + } + + /// + /// Deletes the local branch and the branch on origin per (the shared tail + /// of both and , run once the + /// worktree is gone). A failed local delete throws; a failed remote delete is logged and flagged in the + /// outcome rather than thrown. See for the retry semantics. + /// + private async Task DeleteBranchesAsync( + WorktreeDeletion plan, WorktreeDeletionChoice choice, bool worktreeRemoved, CancellationToken ct) + { + var dir = plan.MainWorktreePath; + bool localDeleted = false, remoteDeleted = false, remoteFailed = false; + if (choice.LocalBranch) { _log($"Deleting local branch '{plan.Branch}'…"); @@ -474,6 +511,40 @@ public async Task DeleteWorktreeAsync( return new WorktreeDeletionOutcome(worktreeRemoved, localDeleted, remoteDeleted, remoteFailed); } + /// + /// Permanently deletes and everything under it — a direct recursive delete, never + /// the Recycle Bin. Clears read-only attributes first (git marks pack files read-only) and, on Windows, runs + /// against an extended-length (\\?\) path so it isn't defeated by the same MAX_PATH limit that stopped + /// git. A folder that's already gone (git removed it partially before failing) is a no-op. + /// + private static void ForceDeleteFolder(string folder) + { + var full = Path.GetFullPath(folder); + if (!Directory.Exists(full)) return; + + var target = ExtendedPath(full); + foreach (var file in Directory.EnumerateFiles(target, "*", SearchOption.AllDirectories)) + { + try { File.SetAttributes(file, FileAttributes.Normal); } + catch { /* best effort — a genuinely locked file will surface on the delete below */ } + } + Directory.Delete(target, recursive: true); + } + + /// + /// On Windows, prefixes a full path with \\?\ (or \\?\UNC\ for a network share) so the Win32 + /// file APIs skip MAX_PATH normalisation — the whole point of the fallback is to remove paths that were + /// already "too long" for git. A no-op on other platforms and when the prefix is already present. + /// + private static string ExtendedPath(string fullPath) + { + if (!OperatingSystem.IsWindows() || fullPath.StartsWith(@"\\?\", StringComparison.Ordinal)) + return fullPath; + return fullPath.StartsWith(@"\\", StringComparison.Ordinal) + ? @"\\?\UNC\" + fullPath.TrimStart('\\') // \\server\share\… → \\?\UNC\server\share\… + : @"\\?\" + fullPath; + } + /// /// Fetches origin/<branch> into the clone so a tracking branch or worktree can be created from /// a branch that exists on the remote but hadn't been fetched yet (see ). diff --git a/src/Services/WorktreeRemovalException.cs b/src/Services/WorktreeRemovalException.cs new file mode 100644 index 0000000..f38455c --- /dev/null +++ b/src/Services/WorktreeRemovalException.cs @@ -0,0 +1,18 @@ +using System; + +namespace Fido.Services; + +/// +/// Thrown by when git worktree remove fails — most +/// often because a path in the worktree is too long for the OS even with git's long-path support. Carries the +/// worktree path so the caller can offer a permanent, Recycle-Bin-bypassing folder delete as a fallback (see +/// ). +/// +public sealed class WorktreeRemovalException : Exception +{ + /// Absolute path of the worktree folder git couldn't remove. + public string WorktreePath { get; } + + public WorktreeRemovalException(string worktreePath, string message) : base(message) + => WorktreePath = worktreePath; +} diff --git a/src/Views/ForceDeleteDialog.axaml b/src/Views/ForceDeleteDialog.axaml new file mode 100644 index 0000000..d113b8a --- /dev/null +++ b/src/Views/ForceDeleteDialog.axaml @@ -0,0 +1,49 @@ + + + + + + + + + + + + + + + + + + + + + private async Task ConfirmAndDeleteWorktreeAsync(string folder, string branch) { @@ -723,7 +724,23 @@ private async Task ConfirmAndDeleteWorktreeAsync(string folder, s if (choice is null || !choice.AnySelected) return TargetOutcome.Cancelled; // backed out or picked nothing — no-op, status cleared by the caller - var outcome = await _opener.DeleteWorktreeAsync(plan, choice); + WorktreeDeletionOutcome outcome; + try + { + outcome = await _opener.DeleteWorktreeAsync(plan, choice); + } + catch (WorktreeRemovalException ex) + { + // git gave up on the folder (usually a path too long). Offer to delete it straight from disk. + _vm.AppendLog($"[✗] git couldn't remove the worktree: {ex.Message}"); + var force = await _dialogs.ConfirmForceDeleteWorktreeFolderAsync(new WorktreeForceDelete(ex.WorktreePath, ex.Message)); + if (!force) + { + _vm.SetStatus($"couldn't remove worktree for '{branch}' — see log", StatusKind.NoGo); + return TargetOutcome.Deleted; // handled: flow ends here, status already set + } + outcome = await _opener.ForceDeleteWorktreeAsync(plan, choice); + } var deleted = new List(); if (outcome.WorktreeRemoved) deleted.Add("worktree"); diff --git a/tests/Fido.Tests/E2E/DeleteWorktreeTests.cs b/tests/Fido.Tests/E2E/DeleteWorktreeTests.cs index c3ae127..e46802a 100644 --- a/tests/Fido.Tests/E2E/DeleteWorktreeTests.cs +++ b/tests/Fido.Tests/E2E/DeleteWorktreeTests.cs @@ -293,6 +293,95 @@ await Harness.WithWindow(services, async window => }); } + [Test] + public async Task When_git_cant_remove_the_worktree_an_accepted_force_delete_completes_it() + { + using var world = new TestRepoWorld(); + var origin = world.CreateOrigin("Foo", "Foo"); + var root = world.SearchRoot("root"); + var clone = world.Clone(origin, root, "Foo"); + var worktree = world.AddWorktree(clone, "feature/x"); + world.PushBranch(worktree, "feature/x"); + + // Make `git worktree remove` fail as if a path were too long; everything else runs against real git. + var git = new GitService((dir, args, ct) => + HasSubcommand(args, "worktree", "remove") + ? Task.FromResult(new ProcessResult(128, "", $"error: unable to unlink '{worktree}/a/very/long/path': Filename too long")) + : ProcessRunner.RunAsync("git", args, dir, ct)); + + var rider = new FakeEditorLauncher(); + var dialogs = new FakeDialogService + { + OnChooser = _ => ChooserDialog.DeleteRequested, + OnConfirmDelete = _ => WorktreeDeletionChoice.All, + OnConfirmForceDelete = _ => true, // accept the disk-level delete + }; + var services = world.BuildServices([root], rider, dialogs, git: git); + + await Harness.WithWindow(services, async window => + { + await window.Open("feature/x"); + Screenshots.Save(window, "D-force-delete-worktree"); + + // The fallback was offered (with the worktree path) and accepted. + await Assert.That(dialogs.ForceDeleteConfirmations.Count).IsEqualTo(1); + await Assert.That(dialogs.ForceDeleteConfirmations[0].WorktreePath).IsEqualTo(Path.GetFullPath(worktree)); + + // The folder, the local branch, and the origin branch are all gone; the flow reports GO. + var check = new GitService(); + await Assert.That(Directory.Exists(worktree)).IsFalse(); + await Assert.That(await check.LocalBranchExistsAsync(clone, "feature/x")).IsFalse(); + await Assert.That(await check.RemoteHasBranchAsync(clone, "feature/x")).IsFalse(); + await Assert.That(window.Vm().StatusKind).IsEqualTo(StatusKind.Go); + }); + } + + [Test] + public async Task When_git_cant_remove_the_worktree_declining_the_force_delete_is_a_no_go() + { + using var world = new TestRepoWorld(); + var origin = world.CreateOrigin("Foo", "Foo"); + var root = world.SearchRoot("root"); + var clone = world.Clone(origin, root, "Foo"); + var worktree = world.AddWorktree(clone, "feature/x"); + + var git = new GitService((dir, args, ct) => + HasSubcommand(args, "worktree", "remove") + ? Task.FromResult(new ProcessResult(128, "", "error: unable to unlink: Filename too long")) + : ProcessRunner.RunAsync("git", args, dir, ct)); + + var rider = new FakeEditorLauncher(); + var dialogs = new FakeDialogService + { + OnChooser = _ => ChooserDialog.DeleteRequested, + OnConfirmDelete = _ => WorktreeDeletionChoice.All, + OnConfirmForceDelete = _ => false, // back out of the disk-level delete + }; + var services = world.BuildServices([root], rider, dialogs, git: git); + + await Harness.WithWindow(services, async window => + { + await window.Open("feature/x"); + + await Assert.That(dialogs.ForceDeleteConfirmations.Count).IsEqualTo(1); // it asked… + + // …and, declined, nothing was deleted — the worktree and its branch remain, status is NO-GO. + var check = new GitService(); + await Assert.That(Directory.Exists(worktree)).IsTrue(); + await Assert.That(await check.LocalBranchExistsAsync(clone, "feature/x")).IsTrue(); + await Assert.That(window.Vm().StatusKind).IsEqualTo(StatusKind.NoGo); + }); + } + + /// True when contains immediately followed by + /// — used to spot the git subcommand under any leading -c key=value flags. + private static bool HasSubcommand(IReadOnlyList args, string first, string second) + { + for (var i = 0; i + 1 < args.Count; i++) + if (args[i] == first && args[i + 1] == second) return true; + return false; + } + [Test] public async Task No_delete_action_is_offered_when_the_branch_sits_in_the_main_tree() { diff --git a/tests/Fido.Tests/Infrastructure/FakeDialogService.cs b/tests/Fido.Tests/Infrastructure/FakeDialogService.cs index 3e2ef3f..5153fe7 100644 --- a/tests/Fido.Tests/Infrastructure/FakeDialogService.cs +++ b/tests/Fido.Tests/Infrastructure/FakeDialogService.cs @@ -27,9 +27,16 @@ public sealed class FakeDialogService : IDialogService /// public Func OnConfirmDelete { get; set; } = _ => null; + /// + /// Force-delete-folder responder used when git couldn't remove the worktree; defaults to declining (the + /// safe default for a Recycle-Bin-bypassing delete). Return true to accept the fallback. + /// + public Func OnConfirmForceDelete { get; set; } = _ => false; + public List ChooserRequests { get; } = new(); public List DecisionRequests { get; } = new(); public List DeleteConfirmations { get; } = new(); + public List ForceDeleteConfirmations { get; } = new(); public int SettingsShownCount { get; private set; } public ChooserRequest? LastChooser => ChooserRequests.Count > 0 ? ChooserRequests[^1] : null; @@ -47,6 +54,12 @@ public sealed class FakeDialogService : IDialogService return Task.FromResult(OnConfirmDelete(plan)); } + public Task ConfirmForceDeleteWorktreeFolderAsync(WorktreeForceDelete request) + { + ForceDeleteConfirmations.Add(request); + return Task.FromResult(OnConfirmForceDelete(request)); + } + public Task ShowDecisionAsync(RepositoryInfo repo, string branch, MainContext context) { var request = new DecisionRequest(repo, branch, context); diff --git a/tests/Fido.Tests/Infrastructure/TestRepoWorld.cs b/tests/Fido.Tests/Infrastructure/TestRepoWorld.cs index 54095c8..5f23677 100644 --- a/tests/Fido.Tests/Infrastructure/TestRepoWorld.cs +++ b/tests/Fido.Tests/Infrastructure/TestRepoWorld.cs @@ -157,7 +157,8 @@ internal FidoServices BuildServices( FakeDialogService dialogs, string? worktreeRoot = null, CloseAfterOpen closeAfterOpen = CloseAfterOpen.CommandLine, - int closeAfterOpenDelaySeconds = 0) + int closeAfterOpenDelaySeconds = 0, + GitService? git = null) { var config = new AppConfig { @@ -179,6 +180,7 @@ internal FidoServices BuildServices( ConfigService = configService, Launcher = launcher, Dialogs = dialogs, + Git = git ?? new GitService(), }; } diff --git a/tests/Fido.Tests/Services/GitServiceTests.cs b/tests/Fido.Tests/Services/GitServiceTests.cs index 23ef1aa..a4aeb01 100644 --- a/tests/Fido.Tests/Services/GitServiceTests.cs +++ b/tests/Fido.Tests/Services/GitServiceTests.cs @@ -142,6 +142,31 @@ public async Task Counts_commits_that_exist_only_on_the_branch() await Assert.That(await git.CountOrphanedCommitsAsync(clone, "feature/x")).IsEqualTo(0); } + [Test] + public async Task Worktree_add_and_remove_pass_gits_long_paths_flag() + { + // Long paths (deep node_modules, generated output) can cross Windows' 260-char MAX_PATH; `-c + // core.longpaths=true` lets git's own file ops handle them when adding or removing a worktree. + var captured = new List>(); + var git = new GitService((_, args, _) => + { + captured.Add(args); + return Task.FromResult(new ProcessResult(0, "", "")); + }); + + await git.WorktreeRemoveAsync("/repo", "/repo.worktrees/x", force: false); + await git.WorktreeRemoveAsync("/repo", "/repo.worktrees/x", force: true); + await git.WorktreeAddExistingAsync("/repo", "/repo.worktrees/x", "feature/x"); + await git.WorktreeAddNewAsync("/repo", "/repo.worktrees/x", "feature/x", startPoint: null); + + await Assert.That(captured.Count).IsEqualTo(4); + foreach (var args in captured) + { + await Assert.That(args[0]).IsEqualTo("-c"); + await Assert.That(args[1]).IsEqualTo("core.longpaths=true"); + } + } + [Test] public async Task Forced_worktree_remove_discards_uncommitted_changes() { diff --git a/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs b/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs index 9c10098..0c1f5a6 100644 --- a/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs +++ b/tests/Fido.Tests/Services/OpenerDeletionRetryTests.cs @@ -33,12 +33,18 @@ private static OpenerService Opener(GitService.GitCommandRunner runner, List args, params string[] prefix) + // Finds the git subcommand as a consecutive run anywhere in the args — so it spots "worktree remove" even + // behind the leading "-c core.longpaths=true" flags that GitService now passes. + private static bool Matches(IReadOnlyList args, params string[] sub) { - if (args.Count < prefix.Length) return false; - for (var i = 0; i < prefix.Length; i++) - if (args[i] != prefix[i]) return false; - return true; + for (var i = 0; i + sub.Length <= args.Count; i++) + { + var all = true; + for (var j = 0; j < sub.Length; j++) + if (args[i + j] != sub[j]) { all = false; break; } + if (all) return true; + } + return false; } [Test] @@ -107,17 +113,18 @@ public async Task Does_not_retry_a_permanent_worktree_removal_failure() return Task.FromResult(new ProcessResult(0, "", "")); }; - InvalidOperationException? thrown = null; + WorktreeRemovalException? thrown = null; try { await Opener(runner).DeleteWorktreeAsync(Plan(), WorktreeDeletionChoice.All); } - catch (InvalidOperationException ex) + catch (WorktreeRemovalException ex) { - thrown = ex; // a permanent worktree-remove failure still throws — nothing has been lost yet + thrown = ex; // a permanent worktree-remove failure throws so the caller can offer a force-delete } await Assert.That(thrown).IsNotNull(); + await Assert.That(thrown!.WorktreePath).IsEqualTo("/repo.worktrees/feature-x"); await Assert.That(removeCalls).IsEqualTo(1); // one attempt, no wasted retries } } diff --git a/tests/Fido.Tests/Services/OpenerForceDeleteTests.cs b/tests/Fido.Tests/Services/OpenerForceDeleteTests.cs new file mode 100644 index 0000000..16e1f9d --- /dev/null +++ b/tests/Fido.Tests/Services/OpenerForceDeleteTests.cs @@ -0,0 +1,77 @@ +using System; +using System.IO; +using System.Linq; +using Fido.Models; +using Fido.Services; +using Fido.Tests.Infrastructure; + +namespace Fido.Tests.Services; + +/// +/// The force-delete fallback used when git worktree remove can't remove the folder (typically a path too +/// long for the OS): deletes the folder straight from disk, +/// prunes git's dangling registration, and finishes the ticked branch deletions. Exercised against a real +/// on-disk git world. +/// +public class OpenerForceDeleteTests +{ + [Test] + public async Task Force_delete_removes_the_folder_prunes_and_deletes_the_branches() + { + using var world = new TestRepoWorld(); + var origin = world.CreateOrigin("Foo", "Foo"); + var root = world.SearchRoot("root"); + var clone = world.Clone(origin, root, "Foo"); + var worktree = world.AddWorktree(clone, "feature/x"); + world.PushBranch(worktree, "feature/x"); + + var git = new GitService(); + var opener = new OpenerService(git, new SolutionFinder(), new WorkingTreeFinder()); + var plan = new WorktreeDeletion( + MainWorktreePath: clone, + WorktreePath: Path.GetFullPath(worktree), + Branch: "feature/x", + RemoteBranchExists: true, + OutstandingChanges: Array.Empty(), + OrphanedCommits: 0); + + var outcome = await opener.ForceDeleteWorktreeAsync(plan, WorktreeDeletionChoice.All); + + // The folder is gone, and the whole delete completed off the back of it. + await Assert.That(Directory.Exists(worktree)).IsFalse(); + await Assert.That(outcome.WorktreeRemoved).IsTrue(); + await Assert.That(outcome.LocalBranchDeleted).IsTrue(); + await Assert.That(outcome.RemoteBranchDeleted).IsTrue(); + await Assert.That(await git.LocalBranchExistsAsync(clone, "feature/x")).IsFalse(); + await Assert.That(await git.RemoteHasBranchAsync(clone, "feature/x")).IsFalse(); + + // Prune cleared the dangling registration — the branch no longer appears as a worktree. + var worktrees = await git.ListWorktreesAsync(clone); + await Assert.That(worktrees.Any(w => w.Branch == "feature/x")).IsFalse(); + } + + [Test] + public async Task Force_delete_of_a_dirty_worktree_honours_keeping_the_local_branch() + { + using var world = new TestRepoWorld(); + var origin = world.CreateOrigin("Foo", "Foo"); + var root = world.SearchRoot("root"); + var clone = world.Clone(origin, root, "Foo"); + var worktree = world.AddWorktree(clone, "feature/x"); + world.MakeDirty(worktree); // uncommitted work — the folder delete removes it regardless + + var git = new GitService(); + var opener = new OpenerService(git, new SolutionFinder(), new WorkingTreeFinder()); + var plan = new WorktreeDeletion(clone, Path.GetFullPath(worktree), "feature/x", + RemoteBranchExists: false, OutstandingChanges: new[] { "?? uncommitted.txt" }, OrphanedCommits: 0); + + // Only the worktree ticked — the local branch should be kept. + var outcome = await opener.ForceDeleteWorktreeAsync( + plan, new WorktreeDeletionChoice(Worktree: true, LocalBranch: false, RemoteBranch: false)); + + await Assert.That(Directory.Exists(worktree)).IsFalse(); + await Assert.That(outcome.WorktreeRemoved).IsTrue(); + await Assert.That(outcome.LocalBranchDeleted).IsFalse(); + await Assert.That(await git.LocalBranchExistsAsync(clone, "feature/x")).IsTrue(); // kept, as chosen + } +} From 54edbd8568b124d153e1b643389a1b8567f016fb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 08:17:16 +0000 Subject: [PATCH 4/4] docs: name the "filename too long" error and surface deletion in the summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the long-path handling discoverable by the exact error users hit ("filename too long" / "unable to unlink … Filename too long") and add a Delete-worktree row to the At-a-glance table. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01KfYxaFc4xQHTt75dhdQoWx --- Docs/Features.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Docs/Features.md b/Docs/Features.md index 9844cce..69ae1fd 100644 --- a/Docs/Features.md +++ b/Docs/Features.md @@ -133,14 +133,16 @@ tidying up a branch you're finished with, in one step: left by a racing git process, or a network blip while deleting the branch on `origin`. Fido retries a few times with a short, backing-off wait — each attempt narrated in the flight log — while **permanent** refusals (`use --force to delete`, `remote ref does not exist`, "not fully merged") still fail fast on the first try. -- **Long paths & a force-delete fallback.** Git's worktree commands run with **long-path support** - (`core.longpaths`), so a worktree whose files cross Windows' **260-character `MAX_PATH`** limit — deep - `node_modules`, generated output — can still be removed. If git **still** can't delete the folder (a path too - long even for that), Fido **offers to delete it straight from disk**: a recursive removal that **bypasses the - Recycle Bin** and uses an extended-length (`\\?\`) path so it isn't defeated by the same limit. Once the - folder is gone Fido **prunes** git's dangling worktree registration and carries on with the branch deletions. - It's an explicit, clearly-labelled confirmation — nothing is force-deleted unless you choose it, and backing - out leaves everything in place. +- **Long filenames & a force-delete fallback.** Deep worktrees can trip Windows' **260-character `MAX_PATH`** + limit — a `node_modules` tree or generated output whose paths are too long — and a delete then fails with + **`filename too long`** / **`unable to unlink … Filename too long`**, leaving the worktree stuck. Fido guards + against this two ways. First, git's worktree commands run with **long-path support** (`core.longpaths`) so + git's own file operations use the Windows extended-length API and can remove those files. Second, if git + **still** can't delete the folder (a path too long even for that), Fido **offers to delete it straight from + disk**: a recursive removal that **bypasses the Recycle Bin** and uses an extended-length (`\\?\`) path so it + isn't defeated by the same limit. Once the folder is gone Fido **prunes** git's dangling worktree registration + and carries on with the branch deletions. It's an explicit, clearly-labelled confirmation — nothing is + force-deleted unless you choose it, and backing out leaves everything in place. - If the remote delete fails for good (say you're offline), the completed **local** cleanup stays done and the failure is reported in the flight log rather than rolled back. @@ -335,6 +337,7 @@ the next save writes to the new location. | Branch-only mode | Open from an existing checkout, or place the branch into a configured repo that already has it | | Cross-clone reuse | Never creates a second worktree for a branch already checked out | | Placement | Switch main tree **or** create a linked worktree | +| Delete worktree | Remove worktree + local/`origin` branch; retries transient failures; long-path aware with a Recycle-Bin-bypassing force-delete for **`filename too long`** | | Open target | `.sln` / `.slnx` / `.slnf` solution, or the repo folder | | Editors | Rider / WebStorm / VS Code / Visual Studio / Zed / Custom — default + Ctrl+1…9, or by CLI slug | | Folder targets | **Console** (`term`) opens a terminal, **File Explorer** (`files`) the OS file manager — Windows / macOS / Linux |