refactor(auth): split writer serialization from reader lock in replaceFileUsers

Add buildMu to serialize replaceFileUsers's writer body (including
buildEntry's potentially slow file-backed resolveSecret reads for
"file:"-prefixed API keys). Scope mu.Lock() to the final byDigest
swap only, so a rebuild in progress no longer stalls every identify
call for its whole duration. Behavior-preserving: writers stay fully
serialized, same as before.

Adds a regression test that holds buildMu externally and asserts
identify still succeeds — proving identify's read path does not
depend on the writer-serialization lock.

auth.go

@@ -83,11 +83,19 @@ type authEntry struct {
 // authStore identifies requests by API key and resolves them to a user and
 // their group. Raw API keys are never retained after construction — only
 // their SHA-256 digests.
+//
+// mu guards byDigest for readers (identify) and the final swap in
+// replaceFileUsers. buildMu serializes the whole body of replaceFileUsers —
+// including buildEntry's potentially slow file-backed secret resolution —
+// so concurrent reload attempts never interleave or overwrite each other.
+// buildMu is never held at the same time as mu, so a rebuild in progress
+// never blocks identify's readers.
 type authStore struct {
 	groups   map[string]*group
 	inline   map[[32]byte]*authEntry // built once by newAuthStore; never mutated afterward
 	byDigest map[[32]byte]*authEntry // inline entries plus the current file-sourced set; guarded by mu
 	mu       sync.RWMutex
+	buildMu  sync.Mutex
 }
 
 // newAuthStore builds an authStore from cfg's groups and inline users. It
@@ -158,14 +166,16 @@ func (a *authStore) buildEntry(uc *UserConfig) (*authEntry, error) {
 // On error the store is left exactly as it was before the call — the new
 // set is validated and built in full before it replaces the old one.
 //
-// mu is held for the whole rebuild, not just the final swap. That serializes
+// buildMu is held for the whole rebuild, not just the final swap — that
-// concurrent callers (a reload timer must never race itself), so one call's
+// serializes concurrent callers (a reload timer must never race itself), so
-// result can never be partially overwritten or interleaved with another's.
+// one call's result can never be partially overwritten or interleaved with
-// identify's reads stay fast (RLock) and reloads are rare, so the extra hold
+// another's. mu, which identify's reads use (RLock), is locked only for the
-// time is a good trade.
+// final map swap below: a slow rebuild (buildEntry's resolveSecret can do
+// blocking file I/O for "file:"-prefixed API keys) no longer stalls every
+// identify call for its whole duration.
 func (a *authStore) replaceFileUsers(us []*UserConfig) error {
-	a.mu.Lock()
+	a.buildMu.Lock()
-	defer a.mu.Unlock()
+	defer a.buildMu.Unlock()
 
 	next := make(map[[32]byte]*authEntry, len(a.inline)+len(us))
 	for digest, entry := range a.inline {
@@ -182,7 +192,9 @@ func (a *authStore) replaceFileUsers(us []*UserConfig) error {
 		next[entry.digest] = entry
 	}
 
+	a.mu.Lock()
 	a.byDigest = next
+	a.mu.Unlock()
 	return nil
 }
 

auth_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http/httptest"
 	"sync"
 	"testing"
+	"time"
 )
 
 // TestIdentify is the brief's Step-1 table: bearer auth resolves the right
@@ -374,3 +375,38 @@ func TestAuthStore_ReplaceFileUsers_ConcurrentWritersAndReaders(t *testing.T) {
 		t.Fatalf("want exactly one file-user batch to survive concurrent swaps (size %d), got size %d — a rebuild was lost or merged with another", want, got)
 	}
 }
+
+// TestAuthStore_Identify_NotBlockedByBuildMu is the regression for the
+// Task-4 lock split: replaceFileUsers's writer-serialization lock (buildMu)
+// must be independent from the reader lock (mu) that identify uses. The
+// prior implementation held mu for the whole rebuild, which would stall
+// every identify call for as long as a reload's build phase (including
+// resolveSecret's potentially slow file-backed reads) took. Holding buildMu
+// externally here simulates "a rebuild's build phase is in progress" — a
+// correct identify must not need buildMu at all.
+func TestAuthStore_Identify_NotBlockedByBuildMu(t *testing.T) {
+	a, err := newAuthStore(testAuthCfg())
+	if err != nil {
+		t.Fatal(err)
+	}
+	a.buildMu.Lock()
+	defer a.buildMu.Unlock()
+
+	r := httptest.NewRequest("POST", "/v1/chat/completions", nil)
+	r.Header.Set("Authorization", "Bearer sk-secret")
+
+	done := make(chan bool, 1)
+	go func() {
+		_, _, ok := a.identify(r)
+		done <- ok
+	}()
+
+	select {
+	case ok := <-done:
+		if !ok {
+			t.Fatal("want identify to succeed")
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("identify blocked while buildMu is held — reader lock must not depend on the writer-serialization lock")
+	}
+}