Fix two RESP3 connect failure modes: credential desync (#3172) and subscription poisoning on downgrade (#3154)

[mgravell]

Two independent bugs in the same area - what happens when a HELLO doesn't go to plan. Both are reproduced by new tests that fail on main. Reviewable commit-by-commit.

1. Never send both HELLO AUTH and a standalone AUTH - fixes #3172

There is a redis server bug behind this one, reproduced on 7.4.10, 8.0.6 and an 8.9 preview build (valkey 8.1.9 is unaffected): inside a pipelined batch, a failing AUTH only gets a reply if it is the first command in that batch. Otherwise the error is silently dropped. Raw socket writes, one batch each, counting replies:

batch	replies
AUTH bad, PING	-WRONGPASS, +PONG (correct)
PING, AUTH bad, PING	+PONG, +PONG (error dropped)
HELLO 3, AUTH bad, PING	<hello map>, +PONG (error dropped)
AUTH bad, AUTH bad, PING	-WRONGPASS, +PONG (second dropped)

It is not HELLO-specific, and it is not "all errors" - other commands' errors are delivered mid-batch fine. Sending the same two commands as separate round trips is also fine, so it is the pipelining path.

Our handshake wrote HELLO <ver> AUTH user pass and a standalone AUTH user pass (the old "we auth EVEN IF we have used HELLO to AUTH" comment). So with credentials the server rejects, the second AUTH's error vanished and every subsequent reply on the connection landed on the wrong message. From a PARSE_DETAIL trace of the handshake: the tie-breaker GET consumed the tracer's ECHO reply, so the tracer never completed, the connection never became usable, and callers saw no connection became available timeouts instead of a clean AuthException. Topology detection read the wrong replies too - which is #3172: a sentinel reporting The ConnectionMultiplexer is not a Sentinel connection. Detected as: Standalone under v3's RESP3 default, cured by forcing protocol=resp2. v2 never hit it because it assumed server version 3.0 and so never issued HELLO. #3140 (separate sentinel credentials) makes "credentials the sentinel rejects" an easy case to land on.

Fix: AUTH goes first in the batch, where its errors are reported correctly, and HELLO follows it bare. HELLO carries credentials only when AUTH is unavailable in the command map - which the ConnectionMultiplexer constructor already only permits when RESP3 is being attempted. Never both.

Verified against the live sentinel topology, same probe either side of the fix:

• before: RESP3 + credentials the sentinel rejects → Detected as: Standalone, no primary connection. Every other protocol/credential combination fine, which is exactly why protocol=resp2 cured it for the reporter.
• after: all four combinations → Sentinel, primary resolved.

ConfigTests.MutableOptions no longer needs Assert.SkipWhen(options.TryResp3(), "only validate RESP2") - that asymmetry was this bug - so it now covers both protocols as a real-server regression test.

2. Reroute subscriptions off the interactive connection on downgrade - fixes #3154

Under RESP3 subscriptions share the interactive connection; under RESP2 they need their own. Which bridge a subscription goes to is decided by what we know-or-expect the protocol to be when it is queued, and that can change underneath a queued message: negotiate RESP3, reconnect later, the new HELLO fails (timeout, expired token, failover to a down-level node), the tracer settles the connection as RESP2. The SUBSCRIBEs that EnsureSubscriptions pushed into the interactive backlog during the disconnect are then written to the interactive connection, putting it into subscriber mode - from then on it rejects every ordinary command:

ERR only [P|S][UN]SUBSCRIBE / PING / QUIT allowed in this context (got: 'SET')

Fix at the write, not at the downgrade: when the interactive bridge is about to write a subscription-bridge message on a connection it now knows is RESP2, it hands the message to the subscription bridge. That covers every drain path (direct write, plus both backlog processors) without having to catch the moment the protocol resolved, and it is robust to any other way a queue outlives the expectation it was filled under. Only that direction is handled: we never upgrade an existing expectation from RESP2 to RESP3, since HELLO is only issued when RESP3 is expected in the first place, so the mirror case cannot arise from a protocol change.

Resp3DowngradeTests drives the whole sequence in-process. Reconnects over the in-proc tunnel complete in ~200ms, so a plain "kill it and re-subscribe" races and routes correctly by luck; the test instead uses the existing OnAcceptClientAsync hook to hold the accept gate shut, giving a real window in which we are disconnected but still expecting RESP3, queues a subscribe into that window, then releases the gate. It also asserts the underlying invariant directly - under RESP2, no single connection may carry both subscription and ordinary commands - which needed a per-connection command record in the test server rather than inferring it from symptoms.

Verification

Full build across all TFMs clean; test suite 5811 passed / 0 failed. Both new tests fail on main with the errors quoted above.

Not in scope

• The redis server bug itself needs filing upstream; we no longer trip it, but any pipelining client can - done (redis 15640)
• The symmetric RESP2 → RESP3 reroute is deliberately absent: I could not construct a path that reaches it.

[mgravell]

thanks @philon-msft - did you see the linked server bug, btw? fun!

[philon-msft]

thanks @philon-msft - did you see the linked server bug, btw? fun!

Yes - I needed a good baffling to start my Friday properly! 🤣