fix: route pull-request mapping through ToHostValue

[matt-edmondson]

Fixes #104

What was wrong

Both providers' pull-request mappers called .As<T>() directly on every host-reported field, violating the invariant CLAUDE.md states as "A field a host reported goes through GitProvider.ToHostValue, never through As<T>() directly." A public hosting method's documented failure surface is the GitHostingException hierarchy, so this raised ArgumentException out of GetPullRequestsAsync and CreatePullRequestAsync, escaping past every catch (GitHostingException) a caller wrote. The sibling ToGitRepository in each file was already correct, so the gap was specific to the pull-request path.

The ?? string.Empty fallbacks on the Azure DevOps side made it worse rather than softer. GitPullRequestTitle and GitBranchName both carry [HasNonWhitespaceContent], so the fallback converted "the host omitted this" into a guaranteed throw of the wrong exception type — on data AzureDevOpsPullRequest.Title itself declares optional ("title": null, or a service-principal IdentityRef whose uniqueName is empty).

Octokit's side is the same shape for a different reason: its model types are mutable classes with unannotated string members deserialized straight from the response, so a field GitHub omits arrives null however non-nullable the property looks. PullRequest.Head and .Base are now dereferenced conditionally for that reason.

The required-vs-nullable decision

The issue flagged this as needing a deliberate answer. Number, Title, SourceBranch and TargetBranch stay required, and a host that omits one raises GitHostingRequestException.

Both hosts document all four as part of what a pull request is, and neither has been observed to omit one; Azure DevOps's DTO declares them string? only because it mirrors the wire shape defensively. Relaxing the model would push a null check onto every caller for a case no host produces, and would turn a host contract violation into a half-populated record that reads as valid. GitProvider.ToRequiredHostValue is the new wrapper that keeps the distinction — ToHostValue for the genuinely optional fields (Author, WebURI, and every GitRepository field), ToRequiredHostValue for these four. No public API shape changes.

Changes

• GitProvider.ToRequiredHostValue<TSemantic> — ToHostValue plus a GitHostingRequestException when the host reported nothing.
• AzureDevOpsProvider.ToGitPullRequest — every field routed; ?? string.Empty fallbacks dropped. StripRefsHeadsPrefix now passes null through instead of having an empty string substituted before it, so an omitted ref stays distinguishable from a reported one.
• GitHubProvider.ToGitPullRequest — every field routed; Head/Base dereferenced conditionally. Now an instance method, since ToHostValue needs Name.
• CLAUDE.md — records the decision and why the ?? string.Empty form was not a softening.

Verification

GitIntegration.Test passes in full: 588 / 588, 0 failures.

The three new throw-asserting tests were confirmed to fail against the unmodified source — reverting only the three source files and re-running leaves the new tests as the only failures, with exactly the wrong exception types the issue describes:

failed TranslatesAnOmittedRequiredPullRequestFieldToGitHostingRequestExceptionAsync (AzureDevOps)
  Expected exception of exact type GitHostingRequestException but caught ArgumentException.
  System.ArgumentException: Cannot convert "" to GitPullRequestTitle
failed TranslatesAnUnrepresentablePullRequestAuthorToGitHostingRequestExceptionAsync (AzureDevOps)
  Expected exception of exact type GitHostingRequestException but caught ArgumentException.
  System.ArgumentException: Cannot convert "" to GitPullRequestAuthor
failed TranslatesAnOmittedRequiredPullRequestFieldToGitHostingRequestExceptionAsync (GitHub)
  Expected exception of exact type GitHostingRequestException but caught ArgumentNullException.
  total: 588, failed: 3, succeeded: 585

Assert.ThrowsExactly is doing the work here: it fails on the ArgumentException the old mapping raised, so these tests cannot pass by accident.

Two further tests pin the other side of the rule — Author stays optional, so a host reporting no identity at all yields null rather than an exception. Without them, routing Author through ToHostValue could be "fixed" by making every field required and nothing would object.

New fixture cases are substitutions over the existing captured payloads (one field varied, every other key exactly as captured), matching how SinglePullRequestListResponse / SinglePullRequestArray already work. No existing test or fixture was altered.

Note: dotnet test reports "Zero tests ran / exit code 5" in this environment, which is unrelated to this change — running the built test executable directly works and is how the numbers above were produced.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C1jqeHWdrdhAoKbN9frz4h

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
8 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 "8 New issues" above — all eight are INFO code smells, and all eight stand as written. Quality gate passed; CI is green on 7bd2fd2 across all three platforms.

They are two suggestions, both flagging conventions these files already use exclusively:

• MSTEST0054 (×5) — TestContext.CancellationToken over TestContext.CancellationTokenSource.Token. The suite uses the latter 234 times across 32 files and the former zero times.
• MSTEST0046 (×3) — Assert.Contains over StringAssert.Contains. AzureDevOpsProviderTests uses StringAssert.Contains 28 times and Assert.Contains zero times; GitHubProviderTests, 18 and zero. (The two also take their arguments in opposite orders, so this is not a mechanical swap.)

Changing only the new lines would leave these two files the only mixed ones in the suite, which is worse than the smell. Both are worth doing as a suite-wide sweep in their own change if the newer idioms are wanted — not worth widening this PR for.

---

Generated by Claude Code