From 038993c8146ed6d1012fce871031d14b3e96957b Mon Sep 17 00:00:00 2001
From: Evan Jarrett <evan@evanjarrett.com>
Date: Sat, 16 May 2026 12:18:06 -0500
Subject: [PATCH] fix concurrency test

---
 pkg/auth/token/handler_concurrency_test.go | 47 +++++++++++++---------
 pkg/auth/token/handler_test.go             | 29 +++++++++++++
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/pkg/auth/token/handler_concurrency_test.go b/pkg/auth/token/handler_concurrency_test.go
index 8754c10..727fe55 100644
--- a/pkg/auth/token/handler_concurrency_test.go
+++ b/pkg/auth/token/handler_concurrency_test.go
@@ -1,10 +1,9 @@
-//go:build !race
-
 package token
 
 import (
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
 )
@@ -12,14 +11,15 @@ import (
 // TestHandler_GateAndFetchRunConcurrently asserts that the authorizer and
 // service-auth fetcher run as parallel goroutines rather than sequentially.
 //
-// Skipped under -race: the race detector inflates RSA-sign cost enough to
-// dominate the wall-clock budget, drowning out the parallel/sequential
-// signal we're trying to measure. The logic being tested isn't race-
-// sensitive — sequential vs. parallel is a static property of the handler
-// — so dropping the timing assertion under race is the right trade.
+// Both stubs share a cross-wired barrier: each closes its own `started`
+// channel and then waits on the other's. If the handler runs them
+// sequentially, the first stub blocks forever waiting for the second — its
+// barrier-timeout fires and the request fails with a 403. If they run in
+// parallel, both goroutines reach the barrier and release each other.
+//
+// This is deterministic — no wall-clock budget — so it works regardless of
+// runner speed or the race detector's overhead.
 func TestHandler_GateAndFetchRunConcurrently(t *testing.T) {
-	const delay = 200 * time.Millisecond
-
 	keyPath := getSharedTestKey(t)
 	issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 5*time.Minute)
 	if err != nil {
@@ -29,28 +29,35 @@ func TestHandler_GateAndFetchRunConcurrently(t *testing.T) {
 	deviceStore, database := setupTestDeviceStore(t)
 	deviceSecret := createTestDevice(t, deviceStore, database, "did:plc:alice123", "alice.bsky.social")
 
+	gateStarted := make(chan struct{})
+	fetchStarted := make(chan struct{})
+	const barrierTimeout = 2 * time.Second
+
 	handler := NewHandler(issuer, deviceStore)
-	handler.SetAuthorizer(&stubAuthorizer{delay: delay})
+	handler.SetAuthorizer(&stubAuthorizer{
+		started:        gateStarted,
+		partnerStarted: fetchStarted,
+		barrierTimeout: barrierTimeout,
+	})
 	handler.SetServiceAuthFetcher(&stubServiceAuthFetcher{
-		expiresAt: time.Now().Add(4 * time.Minute),
-		delay:     delay,
+		expiresAt:      time.Now().Add(4 * time.Minute),
+		started:        fetchStarted,
+		partnerStarted: gateStarted,
+		barrierTimeout: barrierTimeout,
 	})
 
 	req := httptest.NewRequest(http.MethodGet, "/auth/token?service=registry&scope=repository:alice.bsky.social/myapp:pull,push", nil)
 	req.SetBasicAuth("alice", deviceSecret)
 	w := httptest.NewRecorder()
 
-	start := time.Now()
 	handler.ServeHTTP(w, req)
-	elapsed := time.Since(start)
 
 	if w.Code != http.StatusOK {
+		// A barrier-timeout from sequential execution surfaces as a 403 with
+		// the "partner goroutine did not start" message in the body.
+		if strings.Contains(w.Body.String(), "sequential execution") {
+			t.Fatalf("gate and fetch ran sequentially: %s", w.Body.String())
+		}
 		t.Fatalf("expected 200, got %d. Body: %s", w.Code, w.Body.String())
 	}
-	// Sum would be ≥2*delay = 400ms. 350ms gives 150ms slack over max(200ms)
-	// while still flagging sequential execution.
-	if elapsed > 350*time.Millisecond {
-		t.Errorf("handler took %v; gate and fetch should run in parallel (max ~%v, not sum ~%v)",
-			elapsed, delay, 2*delay)
-	}
 }
diff --git a/pkg/auth/token/handler_test.go b/pkg/auth/token/handler_test.go
index 0463a6d..80dc7c0 100644
--- a/pkg/auth/token/handler_test.go
+++ b/pkg/auth/token/handler_test.go
@@ -729,10 +729,26 @@ type stubAuthorizer struct {
 	called bool
 	err    error
 	delay  time.Duration // optional sleep before returning, for parallelism tests
+
+	// Concurrency barrier — when set, Authorize closes `started` then waits on
+	// `partnerStarted`. Two stubs cross-wired this way both block until both
+	// goroutines are live, deterministically proving parallel execution.
+	// If the partner never starts, the wait fails after barrierTimeout.
+	started        chan struct{}
+	partnerStarted chan struct{}
+	barrierTimeout time.Duration
 }
 
 func (s *stubAuthorizer) Authorize(_ context.Context, _, _ string, _ []auth.AccessEntry) error {
 	s.called = true
+	if s.started != nil {
+		close(s.started)
+		select {
+		case <-s.partnerStarted:
+		case <-time.After(s.barrierTimeout):
+			return errors.New("partner goroutine did not start — sequential execution")
+		}
+	}
 	if s.delay > 0 {
 		time.Sleep(s.delay)
 	}
@@ -845,10 +861,23 @@ type stubServiceAuthFetcher struct {
 	expiresAt time.Time
 	err       error
 	delay     time.Duration // optional sleep before returning, for parallelism tests
+
+	// Concurrency barrier — see stubAuthorizer for semantics.
+	started        chan struct{}
+	partnerStarted chan struct{}
+	barrierTimeout time.Duration
 }
 
 func (s *stubServiceAuthFetcher) Fetch(_ context.Context, _, _ string) (time.Time, error) {
 	s.called = true
+	if s.started != nil {
+		close(s.started)
+		select {
+		case <-s.partnerStarted:
+		case <-time.After(s.barrierTimeout):
+			return time.Time{}, errors.New("partner goroutine did not start — sequential execution")
+		}
+	}
 	if s.delay > 0 {
 		time.Sleep(s.delay)
 	}