A partly deleted or corrupt mirror is never re-cloned or reaped, so that repository returns 502 until someone deletes it by hand

[matt-edmondson]

What's wrong

The service never checks that a mirror is a usable repository before trusting it:

• MirrorStore.Exists (GitBranchStateCache/Mirrors/MirrorStore.cs:76) is a bare Directory.Exists. MirrorFetcher (Mirrors/MirrorFetcher.cs:59) re-clones only when that check is false. Otherwise it fetches into whatever is there.
• WriteMarker (MirrorStore.cs:198-209) calls Directory.CreateDirectory(directory) before writing .last-used or .refs-fetched-at inside the mirror directory. MarkUsed runs from MirrorFetcher.cs:204 and BranchStateHandler.cs:169/375, so it can create or recreate the mirror directory on its own.
• The idle reaper's MirrorStore.Delete (MirrorStore.cs:126-132) is a plain recursive delete. It is called from MirrorMaintenanceService and shares no lock with the fetcher.

Failure scenario

1. The idle sweep starts deleting a large mirror.git, and either:
   • the pod is stopped mid-delete (rolling update or eviction), or
   • a request for that repository arrives mid-delete. MarkUsed recreates the directory and .last-used, and the recursive delete fails with "directory not empty". That failure is only logged.
2. mirror.git is now a directory that is not a working bare repository. It may hold nothing but .last-used.
3. Every later request sees Exists == true and fetches instead of cloning. The fetch fails and is treated as stale rather than missing. MarkUsed runs again, and for-each-ref fails, so the request gets 502 refs-unreadable.
4. Each request refreshes .last-used, so the idle sweep never reaps the broken mirror.

The only recovery is to delete the directory by hand on the volume.

Suggested fix

• Reap atomically. Rename the mirror to a tombstone name, for example mirror.git.deleting-<guid>, then delete the tombstone. A crash mid-delete then leaves only a tombstone, which the next sweep can remove.
• Stop markers creating the mirror. WriteMarker should not create the mirror directory. If the directory is gone, skip the marker.
• Validate before trusting. Treat a directory that fails git rev-parse --is-bare-repository (or lacks HEAD/objects) as missing, and re-clone it, into a temporary directory and then rename, as the clone path already does if that applies.

Acceptance criteria

• A mirror directory holding only a .last-used file is re-cloned on the next request, which succeeds.
• A reap that is interrupted, or that races a request, never leaves a mirror.git the fetcher will trust.
• MarkUsed on a missing mirror does not create the directory.
• Tests cover each of these.

[matt-edmondson]

Triage

• Category: Bug
• Priority: High. Once a mirror is in this state, every request for that repository returns 502 until someone deletes the directory by hand. Two things make it self-sustaining: each failing request refreshes .last-used, so the idle sweep never reaps the mirror. It is also easy to trigger, because an ordinary rolling update or eviction during a sweep is enough.
• Area / suggested assignment: mirror lifecycle: MirrorStore (Exists, WriteMarker, Delete), MirrorFetcher, MirrorMaintenanceService
• Duplicates: none. Same repository requested with and without a trailing .git gets two mirrors, two fetches and two cache entries #57 (the same repository with and without a trailing .git) is also in MirrorStore/MirrorFetcher but is a different defect.
• In progress: no matching open PR. The open PRs Bump Testably.Abstractions.FileSystem.Interface from 10.3.0 to 10.4.0 #54 and Bump Testably.Abstractions.Testing from 7.0.2 to 7.1.0 #55 are Testably dependency bumps.

Notes: Of the three fixes, stopping WriteMarker from creating the directory is the smallest, and it closes the race that recreates the directory. Validating a mirror before trusting it is the one that also recovers volumes that are already broken, so it should ship in the same change rather than later.

---

Generated by Claude Code