The PR change as-is would only handle requests without a body, and automatically retry on 1.1.
This has two problems:

• We'll do an additional roundtrip for every successful request
• As soon as you try sending a POST request, you'll be in the same situation as before

Alternatively, we could disable HTTP/2 on the connection pool if we reach this case, so future requests would just start with 1.1. This also has problems:

• If you're talking to an API that only needs session auth in an edge case, you might not love the overhead of ALL requests now avoiding HTTP/2. Maybe that's niche enough to ignore.
• If your first request is POST, it'll still fail since we don't know whether we can retry ourselves.
  • Doesn't make it worse than it already is today. At least if you retry yourself it would start working after the first request.
  • We should be able to fix this in the future by adding some sort of TryReset API on the HttpContent (which I was thinking about for other scenarios already)

I'm leaning towards trying out the alternative approach.

🤖 Copilot Code Review — PR #123827

Holistic Assessment

Motivation: The PR addresses a real problem — NTLM/Negotiate authentication schemes require a persistent, connection-bound handshake that doesn't work over HTTP/2 (which multiplexes streams). When an HTTP/2 response contains a session-based auth challenge, the client needs to retry on HTTP/1.1. This is a genuine gap in the current implementation.

Approach: The PR uses the existing RequestRetryType-based exception/retry pattern (consistent with RetryOnConnectionFailure and RetryOnLowerHttpVersion) to downgrade to HTTP/1.1 on auth challenges. A pool-level flag (_http2SessionAuthSeen) causes future downgradeable requests to skip HTTP/2, while HTTP/2-only requests continue to work. The ALPN negotiation is also correctly adjusted via the new isForHttp2 parameter to prevent TLS from selecting h2 when the intent is HTTP/1.1.

Summary: ⚠️ Needs Human Review. The code is well-structured, follows existing patterns, and has thorough test coverage. However, there are design-scope questions about the pool-wide flag granularity and a minor thread-safety concern that warrant maintainer judgment. (Multi-model review: findings validated by GPT-5.2; Claude Sonnet 4.6 agent timed out.)

Detailed Findings

✅ Retry Pattern — Correct and consistent

The exception-based retry flow follows the existing pattern precisely:

1. Http2Connection.SendAsync detects the auth challenge and throws with RetryOnSessionAuthenticationChallenge
2. SendWithVersionDetectionAndRetryAsync catches it, sets request.Version = HttpVersion.Version11, and loops
3. The exception is fully handled within the pool and never escapes to SendAsyncMultiProxy

The guard conditions are sound: only retry when Content is null (can't rewind streams), VersionPolicy == RequestVersionOrLower (downgrade allowed), and !IsAuthDisabled(). The response is properly disposed before throwing. (Flagged by both models as correct.)

✅ ALPN Negotiation Fix — Important and correct

Threading isForHttp2 into ConnectAsync → GetSslOptionsForRequest ensures that when a downgraded request creates an HTTP/1.1 connection, the TLS ALPN negotiation doesn't advertise h2. Without this, the server could select HTTP/2 via ALPN, defeating the downgrade. This is a critical detail handled correctly.

✅ Test Coverage — Thorough

Six test methods cover the key scenarios:

• Basic downgrade (HTTP/2 → HTTP/1.1) for both NTLM and Negotiate
• Pool flag persistence across subsequent requests
• RequestVersionExact returns 401 as-is (no retry)
• POST with content returns 401 (can't safely retry)
• POST sets pool flag even though it can't retry itself
• HTTP/2-only requests continue working after the flag is set

This covers the major code paths and edge cases well.

⚠️ Pool-wide Downgrade Granularity — Design question for maintainer

_http2SessionAuthSeen is set for any 401 response containing NTLM/Negotiate headers, regardless of the specific path or whether the client intends to authenticate. Once set, all RequestVersionOrLower requests to that origin are routed to HTTP/1.1 for the lifetime of the pool.

This could be overly broad for mixed workloads where only certain endpoints require session auth while others benefit from HTTP/2. For example, a single protected endpoint triggering a 401 would downgrade all anonymous traffic to the same origin.

That said, this mirrors the existing HandleHttp11Downgrade pattern (which sets _http2Enabled = false globally when ALPN fails), and the flag only affects RequestVersionOrLower requests — HTTP/2-only requests still work. The simplicity is a reasonable trade-off, but the maintainer should confirm this pool-wide scope is intentional.

⚠️ Thread Safety of _http2SessionAuthSeen — Benign but worth noting

The field is a plain bool without Volatile.Read/Write. While bool reads/writes are atomic and the flag only transitions false → true (no risk of tearing), there's no memory barrier guaranteeing prompt visibility to concurrent readers. A concurrent request might still attempt HTTP/2, receive another auth challenge, and retry — which is functionally correct but wasteful.

This is consistent with how _http2Enabled is handled elsewhere in the codebase (also a plain bool without volatile), so it follows the established pattern. The commit history shows volatile was explicitly dropped per earlier code review feedback (commit 5728403). Flagging for awareness but not a blocker.

💡 Missing IsAuthDisabled Test Case

There's no test covering the scenario where request.IsAuthDisabled() is true. In that case, the pool flag should still be set (it is), but the request should not retry (it won't). A test for this would document the intended behavior, though the production code handles it correctly.

💡 RequestVersionOrHigher Interaction

A request with Version = 1.1 and VersionPolicy = RequestVersionOrHigher can be routed to HTTP/2 and receive a session auth challenge. In this case, the response is returned as-is (no retry, since the policy isn't RequestVersionOrLower), and the pool flag is set. This seems correct — such a request explicitly opted into version negotiation — but there's no test for this scenario.

Generated by Code Review for issue #123827 · ◷

@copilot Add the "💡 Missing IsAuthDisabled Test Case" suggested by @copilot above

@copilot Add the "💡 Missing IsAuthDisabled Test Case" suggested by @copilot above

Added in 681fdea. The test verifies that when auth is disabled after a redirect (using NetworkCredential, not CredentialCache), the handler returns the 401 response without attempting the HTTP/2→HTTP/1.1 downgrade, even though all other retry conditions are met.