Skip to content

fix: canonicalise the repository path so one repository is one mirror [patch] - #26

Merged
matt-edmondson merged 2 commits into
mainfrom
claude/fix-repository-path-casing
Sep 15, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
claude/fix-repository-path-casing

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #24

What was wrong

The allow-list matches case insensitively on purpose — RepositoryPattern.Translate compiles with RegexOptions.IgnoreCase, and its remarks say why. But every identity derived from the same repositoryPath afterwards compares ordinally:

  • MirrorStore.TryResolve Path.Combines the segments verbatim, so on the Linux container this ships as, Studio/Game.git and studio/game.git are two different mirror.git directories.
  • MirrorKey.ToFlightKey() backs both the SingleFlight clone/fetch coalescing in MirrorFetcher and the Repository component of DiffKey, so the two spellings share neither fetch coalescing nor a DiffCache entry.
  • CredentialAdmission.Key hashes the path ordinally, so each spelling is probed with its own git ls-remote.

Two callers addressing one repository with different casing therefore both pass the allow-list and then cost two bare mirrors on disk, two fetches per polling heartbeat, two computed diffs and two admission probes — the duplication this service exists to remove.

The change

BranchStateHandler.Resolve reduces the repository path to lower case once, immediately after the allow-list accepts it and before it becomes the mirror directory, the flight key, the DiffKey or the admission key. The new Canonicalize helper carries a remark explaining that the allow-list's case-insensitivity is what makes it necessary, so it does not get "simplified" away later, as the triage comment asked.

What is sent to the forge keeps the caller's spelling: the canonical form exists for this service's own bookkeeping, and the forge is the authority on how it spells its own repository names. That also means a deployment against a case-sensitive forge keeps working.

The README gains a short paragraph on this, since the on-disk layout is operator-visible.

Tests

New GitBranchStateCache.Tests/Integration/RepositoryIdentityTests.cs, driving the real handler through the existing ServiceFixture (mock filesystem, scripted git):

  • both spellings are still served (the premise the rest rests on)
  • two spellings share one clone, one ls-remote and one diff-tree
  • two spellings leave one mirror.git directory on the volume
  • a request spelled differently from the allow-list mirrors under the canonical lower-case path

Three of the four fail against main (2 clones, 2 mirror directories, no canonical directory) and pass with the change; verified in both directions by temporarily reverting Canonicalize to the identity function and re-running.

Full suite: 197 passed, 0 failed, in both Debug and Release. Release build is clean, 0 warnings.

Noted, not changed

The upstream key has the same shape of problem — UpstreamRegistry and RepositoryAllowList both look it up OrdinalIgnoreCase, so github and GitHub would split identity the same way. It is left alone here to keep this change to what the issue scopes; worth its own issue if it is judged worth fixing.

🤖 Generated with Claude Code

https://claude.ai/code/session_019AADb2wQHKruFMAnTyzCfv


Generated by Claude Code

matt-edmondson and others added 2 commits September 15, 2026 05:27
… [patch]

The allow-list matches case insensitively on purpose, but every identity
derived from the path afterwards compares ordinally: the mirror directory,
the fetch coalescing key, the diff cache key, and the admission key. Two
callers spelling one repository differently therefore got two bare mirrors
on disk, two fetches per polling heartbeat, two computed diffs, and two
ls-remote probes, which is exactly the duplication this service exists to
remove.

Reduce the path to lower case once, in BranchStateHandler.Resolve, right
after the allow-list accepts it and before anything is derived from it.
What is sent to the forge keeps the caller's spelling, because the forge is
the authority on how it spells its own repository names.

Fixes #24

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019AADb2wQHKruFMAnTyzCfv
The seed was copied out of StateFlowTests, where the string interpolated a
blob id constant. Here the ids are written out, so the $ prefix names no
hole.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019AADb2wQHKruFMAnTyzCfv
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit e53b09f into main Sep 15, 2026
13 checks passed
@matt-edmondson
matt-edmondson deleted the claude/fix-repository-path-casing branch September 15, 2026 08:21
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.

Same repository gets mirrored, fetched and diffed twice when callers vary path casing

1 participant