test(server): deflake reach_rank hook race and the #1008 not-ready window (#224, #227)

cmd/server/index_ready_1008.go

@@ -159,7 +159,11 @@ func (s *PacketStore) maybeCloseIndexReadyCh() {
 // parallel halves the pathHop-not-ready window since the two builders
 // are independent of each other.
 func (s *PacketStore) startBackgroundIndexBuilds() {
+	gate := s.subpathBuildGate // read before the goroutine starts
 	go func() {
+		if gate != nil {
+			gate()
+		}
 		t0 := time.Now()
 		s.mu.Lock()
 		s.buildSubpathIndex()

cmd/server/index_ready_1008_test.go

@@ -14,6 +14,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"sync"
 	"testing"
 	"time"
 )
@@ -50,16 +51,30 @@ func TestIssue1008_PathHopIndexReadyFalseImmediatelyAfterLoad(t *testing.T) {
 
 // TestIssue1008_HandlerReturns503WhileSubpathIndexLoading asserts the
 // analytics/subpaths handler returns 503 + Retry-After: 5 + a JSON body
-// matching the triage spec while the subpath index is still building.
+// matching the triage spec while the subpath index is still building,
+// and 200 once it is built.
+//
+// The background build of the small fixture used to finish before the
+// request under CPU load, so the 503 was a race the test could lose (#227).
+// subpathBuildGate now holds the build until the 503 has been asserted.
 func TestIssue1008_HandlerReturns503WhileSubpathIndexLoading(t *testing.T) {
 	db := setupRichTestDB(t)
 	defer db.Close()
 	store := NewPacketStore(db, nil)
+	release := make(chan struct{})
+	var releaseOnce sync.Once
+	releaseBuild := func() { releaseOnce.Do(func() { close(release) }) }
+	t.Cleanup(releaseBuild) // a failed assertion must not strand the builder
+	building := make(chan struct{})
+	store.subpathBuildGate = func() { close(building); <-release }
 	if err := store.Load(); err != nil {
 		t.Fatalf("Load() error: %v", err)
 	}
-	// Don't wait for the background build — we want to observe the
-	// not-ready window.
+	select {
+	case <-building: // the build has started and is held
+	case <-time.After(5 * time.Second):
+		t.Fatal("background subpath index build never started")
+	}
 	cfg := &Config{}
 	cfg.applyListLimitsDefaults()
 	srv := &Server{store: store, cfg: cfg}
@@ -81,6 +96,17 @@ func TestIssue1008_HandlerReturns503WhileSubpathIndexLoading(t *testing.T) {
 	if body["error"] != "index loading" {
 		t.Errorf(`body["error"] = %v, want "index loading"`, body["error"])
 	}
+
+	// Released, the build completes and the same request is served.
+	releaseBuild()
+	if !store.WaitIndexesReady(5 * time.Second) {
+		t.Fatal("indexes not ready within 5s of releasing the build")
+	}
+	rec = httptest.NewRecorder()
+	srv.handleAnalyticsSubpaths(rec, httptest.NewRequest("GET", "/api/analytics/subpaths?minLen=2&maxLen=4&limit=10", nil))
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status after the build = %d, want 200", rec.Code)
+	}
 }
 
 // TestIssue1008_HandlerRecoversAfterIndexReady asserts that, once the

cmd/server/reach_rank_test.go

@@ -953,6 +953,25 @@ func TestReachRank_ExpiredSnapshotAlwaysRefreshes(t *testing.T) {
 			t.Fatalf("round %d: expired snapshot never refreshed", round)
 		}
 	}
+	awaitDegreeRefresh(t, srv)
+}
+
+// awaitDegreeRefresh waits for the singleflight snapshot refresh a test may
+// have left running (#224). A caller that read the expired snapshot just
+// before the refresh published can register one more refresh before the
+// test's callers return. That refresh runs after the test, finds the
+// snapshot cleared by resetReachState's cleanup, and calls
+// onDegreeSnapshotLoad, unsynchronised with the next test that swaps the
+// hook. Joining the key here waits for it (or, with none left, runs one that
+// returns the fresh snapshot without a load); the receive orders its end
+// before the test returns.
+func awaitDegreeRefresh(t *testing.T, srv *Server) {
+	t.Helper()
+	select {
+	case <-srv.refreshDegreeSnapshot(context.Background()):
+	case <-time.After(10 * time.Second):
+		t.Fatal("snapshot refresh still running 10s after the test")
+	}
 }
 
 // A caller that queued behind a failing rebuild must not start another load

cmd/server/store.go

@@ -309,6 +309,10 @@ type PacketStore struct {
 	// indexReadyCh / maybeCloseIndexReadyCh in index_ready_1008.go.
 	indexReadyChMu sync.Mutex
 	indexReadyChan chan struct{}
+	// subpathBuildGate, nil in production, runs at the start of the
+	// background subpath index build that Load() starts. A test sets it
+	// before Load() to hold the not-ready window open (#227).
+	subpathBuildGate func()
 	// Precomputed distance analytics: hop distances and path totals.
 	// Built LAZILY on first /api/analytics/distance request (#1011) —
 	// previously eager in Load() at startup, which was O(n²) work for