Read Retry-After on GitHub secondary rate limits [patch]

CLAUDE.md

@@ -465,7 +465,9 @@ same 403 to `GitHostingAuthenticationException`. `RateLimitExceededException`,
 `SecondaryRateLimitExceededException`, and `AbuseException` all derive from `ForbiddenException`, so
 **arm order is load-bearing**: the three specific ones must precede it. Only `AbuseException` carries
 a relative delay (`RetryAfterSeconds`) rather than an absolute instant, so it is anchored to
-`DateTimeOffset.UtcNow`; `SecondaryRateLimitExceededException` carries no reset time at all.
+`DateTimeOffset.UtcNow`. `SecondaryRateLimitExceededException` has no reset property of its own, so
+its reset time comes from the response's `Retry-After` header when GitHub sends one, read the same
+way as the bare-429 arm below, and is `null` otherwise.
 
 **A bare `429` needs a fourth arm, matched on status rather than on type.** Octokit has no dedicated
 exception for it: a `429` surfaces as a plain `ApiException`, so it reached

GitIntegration.Test/Hosting/GitHubProviderTests.cs

@@ -771,7 +771,7 @@
 		// Octokit routes a 403 whose body names a secondary rate limit to
 		// SecondaryRateLimitExceededException, which derives from ForbiddenException. It must
 		// therefore be matched before the ForbiddenException arm, or it would be reported as an
-		// authentication failure. GitHub sends no reset time with it, so ResetsAt is null.
+		// authentication failure. This response carries no Retry-After, so ResetsAt is null.
 		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
 			.Respond(
 				(HttpStatusCode)403,
@@ -788,6 +788,35 @@
 		Assert.IsNull(exception.ResetsAt);
 	}
 
+	[TestMethod]
+	public async Task ReadsResetsAtFromRetryAfterOnASecondaryRateLimitResponseAsync()
+	{
+		// SecondaryRateLimitExceededException has no reset property of its own, but GitHub may send
+		// Retry-After with it. Without that delay a caller can only guess when to retry, and GitHub
+		// escalates the limit for clients that retry too early. Bracketed rather than exact, because
+		// the mapping anchors to UtcNow.
+		using FakeHttpMessageHandler handler = new FakeHttpMessageHandler()
+			.Respond(
+				(HttpStatusCode)403,
+				"{\"message\":\"You have exceeded a secondary rate limit\"}",
+				("Content-Type", "application/json"),
+				("Retry-After", "60"));
+		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
+
+		DateTimeOffset before = DateTimeOffset.UtcNow;
+
+		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
+			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
+			.ConfigureAwait(false);
+
+		DateTimeOffset after = DateTimeOffset.UtcNow;
+
+		Assert.AreEqual((HttpStatusCode)403, exception.StatusCode);
+		StringAssert.Contains(exception.ResponseBody, "secondary rate limit");
+		Assert.IsNotNull(exception.ResetsAt);
+		Assert.IsInRange(before.AddSeconds(60), after.AddSeconds(60), exception.ResetsAt.Value);
+	}
+
 	[TestMethod]
 	public async Task TranslatesAnAbuseDetectionResponseToGitHostingRateLimitExceptionAsync()
 	{
@@ -863,7 +892,7 @@
 		GitHubProvider provider = new() { Owner = "contoso".As<GitProviderOwner>(), Handler = handler };
 
 		GitHostingRateLimitException exception = await Assert.ThrowsExactlyAsync<GitHostingRateLimitException>(
 			async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false))
 			.ConfigureAwait(false);
 
 		Assert.AreEqual(HttpStatusCode.TooManyRequests, exception.StatusCode);

GitIntegration/GitHubProvider.cs

@@ -589,7 +589,7 @@
 		return exception switch
 		{
 			RateLimitExceededException rateLimit => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, rateLimit.Reset, exception),
-			SecondaryRateLimitExceededException => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, resetsAt: null, exception),
+			SecondaryRateLimitExceededException => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(TryGetRetryAfterSeconds(exception)), exception),
 			AbuseException abuse => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(abuse.RetryAfterSeconds), exception),
 			{ StatusCode: HttpStatusCode.TooManyRequests } => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(TryGetRetryAfterSeconds(exception)), exception),
 			AuthorizationException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception),
@@ -600,11 +600,12 @@
 	}
 
 	/// <summary>
-	/// Converts an abuse-detection <c>Retry-After</c> delay into the absolute reset time
+	/// Converts a <c>Retry-After</c> delay into the absolute reset time
 	/// <see cref="GitHostingRateLimitException.ResetsAt"/> reports.
 	/// </summary>
 	/// <remarks>
-	/// <see cref="AbuseException.RetryAfterSeconds"/> is a relative delay, and
+	/// <see cref="AbuseException.RetryAfterSeconds"/> and the <c>Retry-After</c> header a secondary
+	/// rate limit or a bare <c>429</c> may carry are relative delays, and
 	/// <see cref="GitHostingRateLimitException.ResetsAt"/> is an absolute instant, because that is
 	/// what <see cref="RateLimitExceededException.Reset"/> and Azure DevOps's
 	/// <c>X-RateLimit-Reset</c> both report. Anchoring to <see cref="DateTimeOffset.UtcNow"/> loses
@@ -682,7 +683,7 @@
 		foreach (KeyValuePair<string, string> header in headers.Where(
 			candidate => candidate.Key.Equals(headerName, StringComparison.OrdinalIgnoreCase)))
 		{
 			foreach (string trimmed in header.Value.Split(';').Select(static parameter => parameter.Trim()))
 			{
 				if (trimmed.StartsWith(urlParameter, StringComparison.OrdinalIgnoreCase))
 				{