Merge pull request #4861 from coo1white/fix-flaky-concurrency-tests

test: stop four concurrency tests from failing on a busy machine

backend/internal/securityaudit/prompt_guard_test.go

@@ -222,7 +222,12 @@ func TestGuardEvaluatorFlagSharedDeadlineFailClosedAndContextCancel(t *testing.T
 		elapsed := time.Since(started)
 		require.Error(t, err)
 		require.Equal(t, 2, calls)
-		require.Less(t, elapsed, 180*time.Millisecond)
+		// The bound only has to prove the failover shared the first endpoint's
+		// 70ms deadline instead of taking the second endpoint's own 500ms one.
+		// An unshared deadline lands at ~535ms, so 350ms still fails loudly
+		// while leaving room for scheduler delay on a busy CI machine. A
+		// tighter bound made this test flaky, not stricter.
+		require.Less(t, elapsed, 350*time.Millisecond)
 		require.GreaterOrEqual(t, elapsed, 50*time.Millisecond)
 		require.Equal(t, int64(1), metrics.Snapshot().Failovers)
 		require.Equal(t, int64(1), metrics.Snapshot().Timeouts)

backend/internal/service/gateway_hotpath_optimization_test.go

@@ -219,7 +219,15 @@ func TestGetUserGroupRateMultiplier_UsesCacheAndSingleflight(t *testing.T) {
 	}
 
 	close(start)
-	time.Sleep(20 * time.Millisecond)
+	// Wait for every caller to have recorded its cache miss before releasing the
+	// loader. A fixed sleep raced here: a goroutine that reached the cache after
+	// the singleflight load had already finished got a hit instead of a miss, and
+	// the miss assertion below saw 11 of 12. The miss counter is the observable
+	// that says "all callers are now inside the singleflight group".
+	require.Eventually(t, func() bool {
+		_, miss, _, _, _ := GatewayUserGroupRateCacheStats()
+		return miss == int64(concurrent)
+	}, 5*time.Second, time.Millisecond, "all callers must miss the cache before the loader is released")
 	close(unblock)
 	wg.Wait()
 

backend/internal/service/ollama_cloud_usage_test.go

@@ -36,6 +36,14 @@ type ollamaUsageTestRepo struct {
 	disableAutoAttempts atomic.Int64
 	disableAutoCalls    atomic.Int64
 	groupResolveCalls   atomic.Int64
+	getByIDCalls        atomic.Int64
+}
+
+// GetByID counts loads so a test can wait for a caller to reach the point just
+// before the singleflight group, instead of guessing with a sleep.
+func (r *ollamaUsageTestRepo) GetByID(ctx context.Context, id int64) (*Account, error) {
+	r.getByIDCalls.Add(1)
+	return r.upstreamBillingProbeAccountRepo.GetByID(ctx, id)
 }
 
 func (r *ollamaUsageTestRepo) ListOllamaCloudUsageGroupAccounts(_ context.Context, anchors []*Account) ([]Account, error) {
@@ -805,7 +813,19 @@ func TestOllamaCloudUsageRefreshSingleflightAndRunnerDeduplicateSharedGroup(t *t
 	errs := make(chan error, 2)
 	go func() { _, err := svc.Refresh(context.Background(), first.ID); errs <- err }()
 	<-started
+	// The first caller is now parked in the stub, having loaded the account twice
+	// (once to build the group key, once inside the singleflight function).
+	loadsBeforeSecond := repo.getByIDCalls.Load()
 	go func() { _, err := svc.Refresh(context.Background(), second.ID); errs <- err }()
+	// Only release the first caller once the second one has loaded its own
+	// account, which happens immediately before it joins the singleflight group.
+	// Releasing right after starting the goroutine raced: if the first refresh
+	// finished first, the second became a fresh singleflight execution, re-read
+	// the account, saw the LastAttemptAt just written, and failed with the 30s
+	// manual-refresh 429 instead of sharing the in-flight result.
+	require.Eventually(t, func() bool {
+		return repo.getByIDCalls.Load() > loadsBeforeSecond
+	}, 5*time.Second, time.Millisecond, "the second caller must reach the singleflight group before the first is released")
 	close(release)
 	require.NoError(t, <-errs)
 	require.NoError(t, <-errs)

backend/internal/service/token_refresh_pool_health_test.go

@@ -630,7 +630,29 @@ func TestTokenRefreshService_SaturatedProviderPreservesConcurrencyAndActualQPSSt
 	starts := refresher.startsSnapshot()
 	require.Len(t, starts, attemptCount)
 	configuredSpacing := time.Second / time.Duration(providerQPS)
-	minimumObservedSpacing := configuredSpacing - 10*time.Millisecond
+
+	// The floor is a tenth of the configured spacing, not spacing minus a few
+	// milliseconds. Each start timestamp is taken after the rate gate releases
+	// the goroutine, so scheduler delay can compress one observed gap without
+	// the gate having done anything wrong: this assertion failed on a loaded
+	// machine at 13ms and again at 37ms against a 40ms floor, both times with
+	// the gate pacing correctly.
+	//
+	// A tenth still separates the two cases by a wide margin. Measured with
+	// providerQPS=20 (50ms spacing), 8 attempts, providerConcurrency=2:
+	//
+	//	gate at 50ms  -> minimum observed gap 49.97ms
+	//	gate at 0     -> minimum observed gap 22µs
+	//
+	// So an unpaced gate lands three orders of magnitude below the 5ms floor
+	// and is still caught, while jitter on a busy machine has room to move.
+	//
+	// Do not swap this for an assertion on the total span of the starts: the
+	// span is dominated by how long each attempt takes under
+	// providerConcurrency, not by the gate. Same measurement — 471ms paced
+	// against 241ms unpaced — so a span check passes with the gate disabled and
+	// tests nothing.
+	minimumObservedSpacing := configuredSpacing / 10
 	actualMinimumSpacing := starts[1].Sub(starts[0])
 	for i := 1; i < len(starts); i++ {
 		spacing := starts[i].Sub(starts[i-1])