Case Study
← All case studiesPaired Implementation Drift in StackExchange.Redis
PR #2995 added cluster subscription routing across dozens of files. The adjudicated logic bug was not a missing test or a swallowed exception — it was inverted boolean polarity between SingleNodeSubscription and MultiNodeSubscription in RemoveDisconnectedEndpoints().
61
files changed
4,039
lines added
1
inverted sibling predicate
What changed
The pull request introduced keyspace notifications and multi-node subscription management. Two sibling classes implement the same cleanup method: RemoveDisconnectedEndpoints(). Single-node mode clears the server when the subscriber is not connected. Multi-node mode was supposed to do the equivalent sweep across servers, but the added condition kept endpoints when IsSubscriberConnected was true — the opposite polarity.
This is a classic copy/paste drift pattern: the method names match, the surrounding feature work is correct, and the bug is one negation away from the reference implementation.
Why this is risky
Wrong endpoint cleanup in a Redis client does not fail loudly at compile time. Under cluster failover or partial disconnects, the library may retain stale server entries or drop live ones. Pub/sub routing becomes nondeterministic relative to connection state, and application code still sees a healthy multiplexer.
Greptile and Qodo surfaced this defect in eval-lab runs. GauntletCI originally missed it while emitting dozens of lower-signal findings on the same large diff.
How GauntletCI catches it now
Rule GCI0058 compares sibling classes in the same diff: same method name, same boolean predicate, opposite polarity. It is deterministic (not LLM-judged) and fires on property patterns like IsSubscriberConnected: false versus .IsSubscriberConnected without a dedicated Redis hardcode.
Platform phases PG-DELIVERY and PG-DOMAIN cap noise on library profiles so this finding survives delivery instead of being buried under per-line fanout.
Why a human reviewer can miss it
Reviewers anchor on the new public API and cluster tests. The bug sits in parallel internal classes that look intentionally symmetric. Without cross-class comparison, the added lines read like reasonable null/state checks rather than an inversion.
Diff evidence
GauntletCI finding
[GCI0058] Paired Implementation Consistency Signal : sibling classes apply opposite polarity to IsSubscriberConnected in the same method Location : src/StackExchange.Redis/Subscription.cs Evidence : SingleNodeSubscription expects IsSubscriberConnected: false; MultiNodeSubscription uses IsSubscriberConnected (true branch) Risk : disconnected endpoints are kept (or connected ones removed) during cluster cleanup Action : align MultiNodeSubscription with SingleNodeSubscription or document intentional divergence
Important context
- GCI0058 currently focuses on if/while/ternary conditions; switch expressions and helper-wrapped predicates may require future hardening.
- The same PR also contains swallowed-handler issues (see the GCI0007 case study on this PR).
- Large feature volume still produces medium-severity edge-case findings (GCI0006) that reviewers should triage separately.
Recommended review steps
- Verify MultiNodeSubscription removes endpoints when IsSubscriberConnected is false, matching SingleNodeSubscription semantics.
- Add a cluster integration test that asserts disconnected servers are evicted from the scratch buffer.
- If polarity is intentional, document the asymmetry in XML remarks on both overrides.
Sources
Eric Cogen -- Founder, GauntletCI
Eric Cogen is a senior .NET engineer with twenty years in production. He has shipped payments systems, internal platforms, and critical line-of-business applications — the kind where a 2 a.m. alert wasn't an emergency, it was a regular Tuesday. GauntletCI is the pre-commit checklist he wishes he had run before every commit.