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

CLAUDE.md

@@ -414,23 +414,45 @@ keyed lookup would work only because `HttpResponseMessage` happens to canonicali
 casing. A `Retry-After` carrying an HTTP date rather than seconds yields `null`, since an unset
 `ResetsAt` is better than an invented one.
 
-**A repository from a hosting provider carries no `LocalPath`, and is addressed by the host's own
-id.** `GitRepository.LocalPath` is nullable because a repository a provider enumerated has never been
+**A repository from a hosting provider carries no `LocalPath`, and how it is addressed is the
+provider's own decision.** `GitRepository.LocalPath` is nullable because a repository a provider enumerated has never been
 cloned. Both providers used to invent one under `Environment.CurrentDirectory`, which made the same
 remote repository yield a different record depending on when it was enumerated, and which forced a
 containment guard to exist purely to keep a name from a remote response from escaping that
 directory. That guard is gone with the reason for it, and `IGitClient.Clone(GitRepository)` now
 reports a missing `LocalPath` rather than resurrecting the invented default — where a working copy
 goes is the caller's decision.
 
-`GitRepository.HostRepositoryId` carries what a host documents its own API in terms of. Microsoft's
+`GitRepository.HostRepositoryId` carries what a host documents its own API in terms of — and the two
+hosts disagree about what that is, which is why the preference is `GitProvider`'s
+`private protected abstract bool PrefersHostRepositoryId` rather than one shared answer. Microsoft's
 reference types Azure DevOps's `{repositoryId}` path parameter as `string (uuid)` and draws an
 explicit id-or-name distinction for the sibling `project` parameter while withholding it here, so
-substituting a name there is unconfirmed against the documented schema rather than sanctioned by it.
-`GetPullRequestsAsync(GitRepository)` and `CreatePullRequest(GitRepository)` therefore prefer the id;
-the `GitRepositoryName`-taking overloads still pass a name, since that is all they are given. Both
-route through one internal core taking the finished path segment, so the choice lives in exactly one
-place and the overloads cannot answer the same question differently.
+substituting a name there is unconfirmed against the documented schema rather than sanctioned by it:
+`AzureDevOpsProvider` prefers the id. GitHub's `{repo}` slot in `/repos/{owner}/{repo}` takes a
+repository **name** only — its id-addressed form is the separate `/repositories/{id}` route — so
+`GitHubProvider` prefers the name. A single shared preference for the id used to be applied to both,
+which put GitHub's numeric id in a slot that only accepts a name and made
+`GetPullRequestsAsync(GitRepository)` and `CreatePullRequest(GitRepository)` answer `404` on GitHub
+for a repository just enumerated from the same provider — the overload `IGitHostingProvider`
+documents as the one to prefer.
+
+The preference only decides which form wins when a `GitRepository` carries both. Which form was
+chosen travels with it, on the internal `GitRepositoryAddress` record struct both internal cores now
+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.
+`AzureDevOpsProvider` has one slot for both forms and ignores the discriminator entirely. The
+`GitRepositoryName`-taking overloads still pass a name, since that is all they are given, and both
+overloads still route through one internal core, so the choice lives in exactly one place and the
+two cannot answer the same question differently.
+
+`FakeHttpMessageHandler.RespondToPath` exists because of this bug: the test that should have caught
+it asserted the wrong URL against a handler that returned `[]` for every path, so the whole method
+ran its success path around a request GitHub would have refused. A path-scoped response answers an
+unexpected path with `404`, the way the host does, which makes a wrong route fail rather than pass
+quietly. Prefer it over plain `Respond` for anything whose route is part of what the test claims.
 
 **A field a host reported goes through `GitProvider.ToHostValue`, never through `As<T>()` directly.**
 The hosting counterpart to `GitParseValues.ToSemantic`, and it exists for the same reason: a value

GitIntegration.Test/Fakes/FakeHttpMessageHandler.cs

@@ -66,7 +66,34 @@ internal sealed class FakeHttpMessageHandler : HttpMessageHandler
 	/// <returns>The same handler, to allow chaining.</returns>
 	public FakeHttpMessageHandler Respond(HttpStatusCode status, string body, params (string Name, string Value)[] headers)
 	{
-		_responses.Enqueue(new QueuedResponse(status, body, headers));
+		_responses.Enqueue(new QueuedResponse(status, body, headers, ExpectedPath: null));
+		_totalQueued++;
+
+		return this;
+	}
+
+	/// <summary>
+	/// Queues the next response, to be returned only if the request that arrives asks for
+	/// <paramref name="expectedPath"/>; any other path is answered <c>404</c>.
+	/// </summary>
+	/// <remarks>
+	/// A route assertion the code under test has to satisfy to get its scripted response at all,
+	/// rather than one made afterwards on a recorded URI. A test that only asserts the URI still
+	/// receives the success body it scripted, so everything downstream of the route — deserialization,
+	/// mapping, the returned model — is exercised against a request the host would have refused. That
+	/// is how a wrong route survived here: a test asserted a URI addressing a repository by an id
+	/// GitHub's name slot cannot accept, and the always-succeeding handler made the whole method look
+	/// correct around it. Answering an unexpected path the way the host would makes the wrong route
+	/// fail where it actually fails.
+	/// </remarks>
+	/// <param name="expectedPath">The <see cref="Uri.AbsolutePath"/> the request must carry, compared exactly.</param>
+	/// <param name="status">The status code the response carries when the path matches.</param>
+	/// <param name="body">The response body, sent as UTF-8 text, when the path matches.</param>
+	/// <param name="headers">Header name/value pairs to attach, routed as <see cref="Respond"/> routes them.</param>
+	/// <returns>The same handler, to allow chaining.</returns>
+	public FakeHttpMessageHandler RespondToPath(string expectedPath, HttpStatusCode status, string body, params (string Name, string Value)[] headers)
+	{
+		_responses.Enqueue(new QueuedResponse(status, body, headers, expectedPath));
 		_totalQueued++;
 
 		return this;
@@ -115,6 +142,21 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 				$"{_totalQueued} queued, {_requests.Count} arrived.");
 		}
 
+		// The scripted response is conditional on the route when the test made it so, and the
+		// condition is checked after the dequeue rather than before it: a request that misses stays
+		// one request, consuming its own queued response, so a following request still gets the
+		// response the test queued for it rather than this one.
+		if (queued.ExpectedPath is string expectedPath && !string.Equals(request.RequestUri!.AbsolutePath, expectedPath, StringComparison.Ordinal))
+		{
+			return new HttpResponseMessage(HttpStatusCode.NotFound)
+			{
+				Content = new StringContent(
+					$"{{\"message\":\"Not Found\",\"expectedPath\":\"{expectedPath}\",\"requestedPath\":\"{request.RequestUri.AbsolutePath}\"}}",
+					Encoding.UTF8,
+					"application/json"),
+			};
+		}
+
 		HttpResponseMessage response = new(queued.Status)
 		{
 			Content = new StringContent(queued.Body, Encoding.UTF8),
@@ -163,7 +205,7 @@ protected override void Dispose(bool disposing)
 		base.Dispose(disposing);
 	}
 
-	private readonly record struct QueuedResponse(HttpStatusCode Status, string Body, (string Name, string Value)[] Headers);
+	private readonly record struct QueuedResponse(HttpStatusCode Status, string Body, (string Name, string Value)[] Headers, string? ExpectedPath);
 
 	/// <summary>A single request this handler received, captured in full.</summary>
 	/// <param name="Method">The HTTP method used.</param>

GitIntegration.Test/Fakes/FakeHttpMessageHandlerTests.cs

@@ -87,6 +87,33 @@
 		StringAssert.Contains(exception.Message, "2 arrived");
 	}
 
+	[TestMethod]
+	public async Task ReturnsAPathScopedResponseOnlyForThatPathAsync()
+	{
+		// The route has to be a condition of getting the scripted response, not merely something a
+		// test inspects afterwards: a handler that answers every path alike lets a wrong route run the
+		// whole success path and look correct, which is how a wrong route shipped here once.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.RespondToPath("/repos/contoso/my-repo/pulls", HttpStatusCode.OK, "[]")
+			.RespondToPath("/repos/contoso/my-repo/pulls", HttpStatusCode.OK, "[]");
+		using HttpClient client = new(handler);
+
+		using HttpResponseMessage matched = await client.GetAsync(new Uri("https://example.invalid/repos/contoso/my-repo/pulls?state=open"), TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
+		using HttpResponseMessage missed = await client.GetAsync(new Uri("https://example.invalid/repos/contoso/90000001/pulls"), TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
+
+		// The query string is no part of the comparison — a route is the path.
+		Assert.AreEqual(HttpStatusCode.OK, matched.StatusCode);
+
+		Assert.AreEqual(HttpStatusCode.NotFound, missed.StatusCode);
+		StringAssert.Contains(
+			await missed.Content.ReadAsStringAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false),
+			"/repos/contoso/90000001/pulls");
+
+		// A missed request still consumed its own queued response rather than the next one's, so a
+		// following request is answered by what the test queued for it.
+		Assert.AreEqual(2, handler.Requests.Count);
+	}
+
 	[TestMethod]
 	public async Task AttachesResponseHeadersWhenGivenAsync()
 	{

GitIntegration.Test/Hosting/GitHubProviderTests.cs

@@ -174,13 +174,15 @@
 	}
 
 	[TestMethod]
-	public async Task AddressesARepositoryByItsHostIdWhenListingPullRequestsAsync()
+	public async Task AddressesARepositoryByNameWhenListingPullRequestsAsync()
 	{
-		// The point of carrying the id: it has to reach the request path. GitHub happens to accept a
-		// name in this position, but both providers answer the same question the same way under one
-		// interface, so both prefer the host's own identifier when one is known.
+		// GET /repos/{owner}/{repo}/pulls takes a repository NAME in its {repo} slot — the id-addressed
+		// form is the separate /repositories/{id}/pulls. This provider used to send GitHub's numeric
+		// HostRepositoryId here, which addresses no repository at all and answers 404 for one that was
+		// just enumerated from the same provider. The repository below carries both forms, so this is
+		// the case where the preference itself decides, and the name has to win.
 		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
-			.Respond(HttpStatusCode.OK, "[]", ("Content-Type", "application/json"));
+			.RespondToPath("/repos/contoso/my-repo/pulls", HttpStatusCode.OK, "[]", ("Content-Type", "application/json"));
 		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
 
 		GitRepository repository = new()
@@ -191,7 +193,71 @@
 
 		_ = await provider.GetPullRequestsAsync(repository, TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
 
-		StringAssert.Contains(handler.Requests[0].Uri.AbsoluteUri, "/repos/contoso/90000001/pulls");
+		Assert.AreEqual("/repos/contoso/my-repo/pulls", handler.Requests[0].Uri.AbsolutePath);
+	}
+
+	[TestMethod]
+	public async Task AddressesARepositoryByItsHostIdOnTheIdRouteWhenNoNameIsKnownAsync()
+	{
+		// Preferring the name does not discard the id. A repository carrying only a HostRepositoryId —
+		// one a caller built by hand, since GetRepositoriesAsync always reports a name — is addressed
+		// on GitHub's id-addressed route rather than by dropping its id into the name slot.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.RespondToPath("/repositories/90000001/pulls", HttpStatusCode.OK, "[]", ("Content-Type", "application/json"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		GitRepository repository = new() { HostRepositoryId = "90000001".As<GitHostRepositoryId>() };
+
+		_ = await provider.GetPullRequestsAsync(repository, TestContext.CancellationTokenSource.Token).ConfigureAwait(false);
+
+		Assert.AreEqual("/repositories/90000001/pulls", handler.Requests[0].Uri.AbsolutePath);
+	}
+
+	[TestMethod]
+	public async Task ReportsAHostRepositoryIdGitHubCannotBeAddressedByAsync()
+	{
+		// GitHostRepositoryId is unvalidated, because Azure DevOps's is a uuid and GitHub's a whole
+		// number, so the semantic type can enforce neither. A hand-built repository carrying a
+		// GitHub-impossible id is a caller's argument rather than anything a host reported — nothing is
+		// sent, so there is no response for a GitHostingException to describe.
+		using FakeHttpMessageHandler handler = new();
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		GitRepository repository = new() { HostRepositoryId = "not-a-number".As<GitHostRepositoryId>() };
+
+		ArgumentException exception = await Assert.ThrowsExactlyAsync<ArgumentException>(
+			async () => await provider.GetPullRequestsAsync(repository, TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
+			.ConfigureAwait(false);
+
+		StringAssert.Contains(exception.Message, "whole numbers");
+		Assert.AreEqual(0, handler.Requests.Count);
+	}
+
+	[TestMethod]
+	public async Task AddressesARepositoryByNameWhenCreatingAPullRequestAsync()
+	{
+		// CreatePullRequest(GitRepository) reaches GitHub's routes the same way GetPullRequestsAsync
+		// does, and carried the identical bug: Octokit's Create(owner, name, ...) builds
+		// POST /repos/{owner}/{repo}/pulls, so a numeric id in that slot 404s just as it does on the
+		// listing route.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.RespondToPath("/repos/contoso/my-repo/pulls", HttpStatusCode.Created, Fixture("github-pullrequest-created.json"), ("Content-Type", "application/json"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		GitRepository repository = new()
+		{
+			Name = "my-repo".As<GitRepositoryName>(),
+			HostRepositoryId = "90000001".As<GitHostRepositoryId>(),
+		};
+
+		_ = await provider.CreatePullRequest(repository)
+			.From("example-branch-1".As<GitBranchName>())
+			.Into("main".As<GitBranchName>())
+			.Titled("A title".As<GitPullRequestTitle>())
+			.ExecuteAsync(TestContext.CancellationTokenSource.Token)
+			.ConfigureAwait(false);
+
+		Assert.AreEqual("/repos/contoso/my-repo/pulls", handler.Requests[0].Uri.AbsolutePath);
 	}
 
 	[TestMethod]

GitIntegration.Test/Hosting/GitProviderTests.cs

@@ -215,10 +215,15 @@ private sealed class TestProvider : GitProvider
 		public override Task<IReadOnlyList<GitRepository>> GetRepositoriesAsync(CancellationToken cancellationToken = default) =>
 			throw new NotSupportedException("Not exercised by these tests.");
 
-		internal override Task<IReadOnlyList<GitPullRequest>> GetPullRequestsCoreAsync(string repositoryIdentifier, CancellationToken cancellationToken) =>
+		// Arbitrary, because this provider issues no request for the choice to matter. The member is
+		// abstract so that each real provider has to state its own host's answer rather than inherit
+		// one that is wrong for it — GitHub and Azure DevOps genuinely differ.
+		private protected override bool PrefersHostRepositoryId => false;
+
+		internal override Task<IReadOnlyList<GitPullRequest>> GetPullRequestsCoreAsync(GitRepositoryAddress repositoryAddress, CancellationToken cancellationToken) =>
 			throw new NotSupportedException("Not exercised by these tests.");
 
-		internal override Task<GitPullRequest> CreatePullRequestCoreAsync(string repositoryIdentifier, GitPullRequestSpecification specification, CancellationToken cancellationToken) =>
+		internal override Task<GitPullRequest> CreatePullRequestCoreAsync(GitRepositoryAddress repositoryAddress, GitPullRequestSpecification specification, CancellationToken cancellationToken) =>
 			throw new NotSupportedException("Not exercised by these tests.");
 
 		public HostingCredential CallResolveCredential() => ResolveCredential();

GitIntegration.Test/Hosting/GitPullRequestCreateBuilderTests.cs

@@ -192,12 +192,17 @@ private sealed class RecordingProvider : GitProvider
 		public override Task<IReadOnlyList<GitRepository>> GetRepositoriesAsync(CancellationToken cancellationToken = default) =>
 			throw new NotSupportedException("Not exercised by the routing test.");
 
-		internal override Task<IReadOnlyList<GitPullRequest>> GetPullRequestsCoreAsync(string repositoryIdentifier, CancellationToken cancellationToken) =>
+		// Arbitrary, because this provider issues no request for the choice to matter. The member is
+		// abstract so that each real provider has to state its own host's answer rather than inherit
+		// one that is wrong for it — GitHub and Azure DevOps genuinely differ.
+		private protected override bool PrefersHostRepositoryId => false;
+
+		internal override Task<IReadOnlyList<GitPullRequest>> GetPullRequestsCoreAsync(GitRepositoryAddress repositoryAddress, CancellationToken cancellationToken) =>
 			throw new NotSupportedException("Not exercised by the routing test.");
 
-		internal override Task<GitPullRequest> CreatePullRequestCoreAsync(string repositoryIdentifier, GitPullRequestSpecification specification, CancellationToken cancellationToken)
+		internal override Task<GitPullRequest> CreatePullRequestCoreAsync(GitRepositoryAddress repositoryAddress, GitPullRequestSpecification specification, CancellationToken cancellationToken)
 		{
-			Repository = repositoryIdentifier;
+			Repository = repositoryAddress.Value;
 			Specification = specification;
 			return Task.FromResult(CannedPullRequest);
 		}