fix: address a GitHub repository by name, and an id on GitHub's id route

[matt-edmondson]

Fixes #101

The bug

GitProvider.ToRepositoryIdentifier preferred HostRepositoryId over Name for every provider. That is what Azure DevOps's documented {repositoryId} schema asks for, but it is wrong for GitHub: the {repo} slot in /repos/{owner}/{repo} takes a repository name, and the id-addressed form is the separate /repositories/{id} route.

GitHubProvider.ToGitRepository fills HostRepositoryId with GitHub's numeric id, so

IReadOnlyList<GitRepository> repos = await gitHub.GetRepositoriesAsync();
await gitHub.GetPullRequestsAsync(repos[0]);

issued GET /repos/contoso/90000001/pulls → 404 → GitHostingNotFoundException, for a repository just enumerated from the same provider. CreatePullRequest(GitRepository) had the identical problem. Both are the overload IGitHostingProvider documents as "the one to prefer when the caller has a GitRepository from GetRepositoriesAsync", so the recommended call sequence was broken end to end on GitHub.

The fix

• The preference is now per-provider: GitProvider.PrefersHostRepositoryId is private protected abstract. Azure DevOps answers true (its one {repositoryId} slot is documented as taking the id); GitHub answers false.
• The preference only decides which form wins when a GitRepository carries both. Which form was chosen now travels with the value on the internal GitRepositoryAddress record struct that both internal cores take, so a repository carrying only an id is still addressed correctly on GitHub — through Octokit's long repositoryId overloads, which build the id-addressed route — rather than by dropping an id into the name slot. A bare string could not carry that distinction, and a provider cannot recover it from the value's shape, since a repository may legitimately be named a number.
• Azure DevOps has one slot for both forms and ignores the discriminator; its behaviour is unchanged.
• A GitHostRepositoryId GitHub cannot be addressed by (the type is unvalidated, because Azure DevOps's is a uuid and GitHub's a whole number) raises ArgumentException before anything is sent, rather than a GitHostingException with no response to describe.

No public API changed — GitRepositoryAddress and the two core methods are internal.

Why the tests did not catch it

GitHubProviderTests asserted the URL /repos/contoso/90000001/pulls against a fake handler that returned [] regardless of path, so the whole method ran its success path around a request GitHub would have refused.

FakeHttpMessageHandler.RespondToPath makes the route a condition of getting the scripted response, answering an unexpected path with 404 the way the host does. The listing test now asserts the name-addressed URL through it, joined by tests for the id-route and create-route cases.

Verification

• dotnet build clean in Debug and Release, zero warnings, no suppressions added.
• dotnet test: 583 passed, 0 failed.
• Mutation-checked by substitution (per CLAUDE.md), re-verifying the tree after each run:
  • PrefersHostRepositoryId => true on GitHub → AddressesARepositoryByNameWhenListingPullRequestsAsync and AddressesARepositoryByNameWhenCreatingAPullRequestAsync fail.
  • the route discriminator forced to false → AddressesARepositoryByItsHostIdOnTheIdRouteWhenNoNameIsKnownAsync and ReportsAHostRepositoryIdGitHubCannotBeAddressedByAsync fail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TU2gC1LwTwyMin2XxYxg2m

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[matt-edmondson]

On SonarCloud's 10 New issues above — the gate passed, and all ten are INFO-severity external_roslyn MSTest suggestions on new test lines, each flagging this repository's established idiom rather than anything this change introduced:

Rule	Count	Suggests	Uses in GitIntegration.Test source today
MSTEST0054	6	TestContext.CancellationToken	TestContext.CancellationTokenSource.Token × 229
MSTEST0046	2	Assert.Contains	StringAssert.Contains × 69
MSTEST0037	2	Assert.IsEmpty / Assert.HasCount	neither appears anywhere in source

They surface now only because these lines are new; the analyzers are not enabled as build warnings, and both Debug and Release build with zero warnings. Adopting the suggestions on these few lines alone would leave two idioms in files that consistently use one, so the new tests stay matched to their surroundings.

Happy to sweep any of the three repo-wide as its own change if that is wanted — that seems the right shape for it, rather than a partial conversion riding this fix.

---

Generated by Claude Code