From b43257d69a0e9fdf6002d5ef513774a44520d2b6 Mon Sep 17 00:00:00 2001 From: Ryan Mello Date: Sun, 26 Jul 2026 08:38:24 +0100 Subject: [PATCH] fix(model): let loadLocationStables honour forceMinimum The read side hardcoded forceMinimum=false, so a location where every provider fails the minimums gate was invisible even to callers that explicitly bypass minimums. Providers there could never be probed and so could never graduate probation. The writer already populates both key families; this only chooses between them. User-facing listing still passes false. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg --- model/network_client_location_model.go | 13 ++- model/network_client_location_model_test.go | 101 +++++++++++++++++++- 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/model/network_client_location_model.go b/model/network_client_location_model.go index d753a666..62344ca8 100644 --- a/model/network_client_location_model.go +++ b/model/network_client_location_model.go @@ -1858,6 +1858,13 @@ func loadInitialClientLocations(ctx context.Context) (initialClientLocations *In func loadLocationStables( ctx context.Context, locationIds []server.Id, + // forceMinimum selects which pre-computed key family to read. The writer + // (UpdateClientScores) populates both, so this only chooses between them. + // User-facing listing passes false and keeps today's behaviour; an operator + // census passes true, because a location where every provider fails the + // minimums gate is otherwise invisible and its providers can never be + // probed or graduate probation. + forceMinimum bool, rankMode RankMode, clientLocationId server.Id, ) ( @@ -1874,7 +1881,7 @@ func loadLocationStables( for _, locationId := range locationIds { locationFilterCmds[locationId] = pipe.Get( ctx, - clientScoreLocationFilterKey(false, rankMode, locationId, clientLocationId), + clientScoreLocationFilterKey(forceMinimum, rankMode, locationId, clientLocationId), ) } // note ignore the error for GET since it will include missing key @@ -1990,6 +1997,8 @@ func FindProviderLocations( locationStables, _ := loadLocationStables( session.Ctx, slices.Collect(maps.Keys(clientLocations)), + // user-facing search: only surface locations that meet the bar + false, rankMode, clientLocationId, ) @@ -2075,6 +2084,8 @@ func GetProviderLocations( locationStables, _ := loadLocationStables( session.Ctx, locationIds, + // user-facing listing: only surface locations that meet the bar + false, rankMode, clientLocationId, ) diff --git a/model/network_client_location_model_test.go b/model/network_client_location_model_test.go index 450554d3..f253911f 100644 --- a/model/network_client_location_model_test.go +++ b/model/network_client_location_model_test.go @@ -705,6 +705,7 @@ func TestClientLocationScoreCacheRoundTrip(t *testing.T) { locationStables, err := loadLocationStables( ctx, []server.Id{city.LocationId, city.RegionLocationId, city.CountryLocationId}, + false, RankModeQuality, usLocationId, ) @@ -1378,7 +1379,7 @@ func TestUpdateClientScoresCountsClientsWithoutReliabilityScores(t *testing.T) { err = UpdateClientScores(ctx, time.Hour, 1) connect.AssertEqual(t, err, nil) - locationStables, err := loadLocationStables(ctx, []server.Id{city.CountryLocationId}, RankModeQuality, server.Id{}) + locationStables, err := loadLocationStables(ctx, []server.Id{city.CountryLocationId}, false, RankModeQuality, server.Id{}) connect.AssertEqual(t, err, nil) _, ok := locationStables[city.CountryLocationId] connect.AssertEqual(t, ok, true) @@ -1559,6 +1560,7 @@ func TestUpdateClientScoresCountsOnlyPublicProviders(t *testing.T) { locationStables, err := loadLocationStables( ctx, []server.Id{publicCity.CountryLocationId, networkOnlyCity.CountryLocationId}, + false, RankModeQuality, server.Id{}, ) @@ -1749,3 +1751,100 @@ func connectPublicAndNetworkOnlyProviders( }) return } + +// the prober enumerates locations through `GET /network/provider-locations` -> +// `GetProviderLocations` -> `loadLocationStables`, and only then asks +// `find-providers2` for the providers at each one. `force_minimum` on that +// second call cannot recover a location the first call never listed, and the +// read side hardcoded the `forceMinimum=false` filter key -- so a location +// whose every provider fails the minimums gate was invisible, and those +// providers could never be probed and so could never graduate probation. +// +// One connected+valid Public provider with no latency or speed samples, which +// deterministically fails the strict minimums. The writer populates both key +// families, so the same location must be absent under `forceMinimum=false` +// (today's user-facing behaviour, unchanged) and present under `true`. +func TestLoadLocationStablesHonoursForceMinimum(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + networkId := server.NewId() + userId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + + clientSession := session.Testing_CreateClientSession( + ctx, + jwt.NewByJwt(networkId, userId, "a", false, false), + ) + + handlerId := CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, "0.0.0.1:0", handlerId) + connect.AssertEqual(t, err, nil) + + err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + + // a provider a stranger can actually use, so the provide-mode filter is + // not what excludes it + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + + // a lookback_index = 0 reliability score row, so the source query's join + // against `client_connection_reliability_score` is not what excludes it + // either. deliberately no `network_client_latency` / `network_client_speed` + // rows: the missing latency and speed tests are the only reason this + // provider fails the minimums. + clientAddressHash, _, err := clientSession.ClientAddressHashPort() + connect.AssertEqual(t, err, nil) + AddClientReliabilityStats( + ctx, + networkId, + clientId, + clientAddressHash, + server.NowUtc(), + &ClientReliabilityStats{ + ConnectionEstablishedCount: 1, + ProvideEnabledCount: 1, + ReceiveMessageCount: 1, + ReceiveByteCount: 1024, + SendMessageCount: 1, + SendByteCount: 1024, + }, + ) + UpdateClientReliabilityScores(ctx, server.NowUtc(), true) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + err = UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + locationIds := []server.Id{city.CountryLocationId} + + // user-facing listing: the provider is below the bar, so the location + // has no entry at all -- a missing entry is how loadLocationStables says + // "no providers" + strict, err := loadLocationStables(ctx, locationIds, false, RankModeQuality, server.Id{}) + connect.AssertEqual(t, err, nil) + _, ok := strict[city.CountryLocationId] + connect.AssertEqual(t, ok, false) + + // operator census: the same location must be listed, otherwise its + // providers can never be reached + forced, err := loadLocationStables(ctx, locationIds, true, RankModeQuality, server.Id{}) + connect.AssertEqual(t, err, nil) + _, ok = forced[city.CountryLocationId] + connect.AssertEqual(t, ok, true) + }) +}