add makefile fix race conditions

.gitignore

@@ -11,6 +11,11 @@ dist/
 # Environment configuration
 .env
 
+# Generated assets (run go generate to rebuild)
+pkg/appview/licenses/spdx-licenses.json
+pkg/appview/static/js/htmx.min.js
+pkg/appview/static/js/lucide.min.js
+
 # IDE
 .claude/
 .vscode/

Makefile

@@ -0,0 +1,84 @@
+# ATCR Makefile
+# Build targets for the ATProto Container Registry
+
+.PHONY: all build build-appview build-hold build-credential-helper build-oauth-helper \
+	generate test test-race test-verbose lint clean help
+
+.DEFAULT_GOAL := help
+
+help: ## Show this help message
+	@echo "ATCR Build Targets:"
+	@echo ""
+	@awk 'BEGIN {FS = ":.*##"; printf ""} /^[a-zA-Z_-]+:.*?##/ { printf "  \033[36m%-28s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+
+all: generate build ## Generate assets and build all binaries (default)
+
+# Generated asset files
+GENERATED_ASSETS = \
+	pkg/appview/static/js/htmx.min.js \
+	pkg/appview/static/js/lucide.min.js \
+	pkg/appview/licenses/spdx-licenses.json
+
+generate: $(GENERATED_ASSETS) ## Run go generate to download vendor assets
+
+$(GENERATED_ASSETS):
+	@echo "→ Generating vendor assets and code..."
+	go generate ./...
+
+##@ Build Targets
+
+build: build-appview build-hold build-credential-helper ## Build all binaries
+
+build-appview: $(GENERATED_ASSETS) ## Build appview binary only
+	@echo "→ Building appview..."
+	@mkdir -p bin
+	go build -o bin/atcr-appview ./cmd/appview
+
+build-hold: $(GENERATED_ASSETS) ## Build hold binary only
+	@echo "→ Building hold..."
+	@mkdir -p bin
+	go build -o bin/atcr-hold ./cmd/hold
+
+build-credential-helper: $(GENERATED_ASSETS) ## Build credential helper only
+	@echo "→ Building credential helper..."
+	@mkdir -p bin
+	go build -o bin/docker-credential-atcr ./cmd/credential-helper
+
+build-oauth-helper: $(GENERATED_ASSETS) ## Build OAuth helper only
+	@echo "→ Building OAuth helper..."
+	@mkdir -p bin
+	go build -o bin/oauth-helper ./cmd/oauth-helper
+
+##@ Test Targets
+
+test: ## Run all tests
+	@echo "→ Running tests..."
+	go test -cover ./...
+
+test-race: ## Run tests with race detector
+	@echo "→ Running tests with race detector..."
+	go test -race ./...
+
+test-verbose: ## Run tests with verbose output
+	@echo "→ Running tests with verbose output..."
+	go test -v ./...
+
+##@ Quality Targets
+
+.PHONY: check-golangci-lint
+check-golangci-lint:
+	@which golangci-lint > /dev/null || (echo "→ Installing golangci-lint..." && go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest)
+
+lint: check-golangci-lint ## Run golangci-lint
+	@echo "→ Running golangci-lint..."
+	golangci-lint run ./...
+
+##@ Utility Targets
+
+clean: ## Remove built binaries and generated assets
+	@echo "→ Cleaning build artifacts..."
+	rm -rf bin/
+	rm -f pkg/appview/static/js/htmx.min.js
+	rm -f pkg/appview/static/js/lucide.min.js
+	rm -f pkg/appview/licenses/spdx-licenses.json
+	@echo "✓ Clean complete"

pkg/appview/storage/context_test.go

@@ -2,6 +2,7 @@ package storage
 
 import (
 	"context"
+	"sync"
 	"testing"
 
 	"atcr.io/pkg/atproto"
@@ -9,20 +10,37 @@ import (
 
 // Mock implementations for testing
 type mockDatabaseMetrics struct {
+	mu        sync.Mutex
 	pullCount int
 	pushCount int
 }
 
 func (m *mockDatabaseMetrics) IncrementPullCount(did, repository string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.pullCount++
 	return nil
 }
 
 func (m *mockDatabaseMetrics) IncrementPushCount(did, repository string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.pushCount++
 	return nil
 }
 
+func (m *mockDatabaseMetrics) getPullCount() int {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.pullCount
+}
+
+func (m *mockDatabaseMetrics) getPushCount() int {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.pushCount
+}
+
 type mockReadmeCache struct{}
 
 func (m *mockReadmeCache) Get(ctx context.Context, url string) (string, error) {

pkg/appview/storage/manifest_store.go

@@ -11,6 +11,7 @@ import (
 	"maps"
 	"net/http"
 	"strings"
+	"sync"
 	"time"
 
 	"atcr.io/pkg/atproto"
@@ -22,6 +23,7 @@ import (
 // It stores manifests in ATProto as records
 type ManifestStore struct {
 	ctx                *RegistryContext       // Context with user/hold info
+	mu                 sync.RWMutex           // Protects lastFetchedHoldDID
 	lastFetchedHoldDID string                 // Hold DID from most recently fetched manifest (for pull)
 	blobStore          distribution.BlobStore // Blob store for fetching config during push
 }
@@ -67,6 +69,7 @@ func (s *ManifestStore) Get(ctx context.Context, dgst digest.Digest, options ...
 	// Store the hold DID for subsequent blob requests during pull
 	// Prefer HoldDID (new format) with fallback to HoldEndpoint (legacy URL format)
 	// The routing repository will cache this for concurrent blob fetches
+	s.mu.Lock()
 	if manifestRecord.HoldDID != "" {
 		// New format: DID reference (preferred)
 		s.lastFetchedHoldDID = manifestRecord.HoldDID
@@ -74,6 +77,7 @@ func (s *ManifestStore) Get(ctx context.Context, dgst digest.Digest, options ...
 		// Legacy format: URL reference - convert to DID
 		s.lastFetchedHoldDID = atproto.ResolveHoldDIDFromURL(manifestRecord.HoldEndpoint)
 	}
+	s.mu.Unlock()
 
 	var ociManifest []byte
 
@@ -232,6 +236,8 @@ func digestToRKey(dgst digest.Digest) string {
 // GetLastFetchedHoldDID returns the hold DID from the most recently fetched manifest
 // This is used by the routing repository to cache the hold for blob requests
 func (s *ManifestStore) GetLastFetchedHoldDID() string {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
 	return s.lastFetchedHoldDID
 }
 

pkg/appview/storage/manifest_store_test.go

@@ -669,13 +669,13 @@ func TestManifestStore_Get_OnlyCountsGETRequests(t *testing.T) {
 
 			if tt.expectPullIncrement {
 				// Check that IncrementPullCount was called
-				if mockDB.pullCount == 0 {
+				if mockDB.getPullCount() == 0 {
 					t.Error("Expected pull count to be incremented for GET request, but it wasn't")
 				}
 			} else {
 				// Check that IncrementPullCount was NOT called
-				if mockDB.pullCount > 0 {
-					t.Errorf("Expected pull count NOT to be incremented for %s request, but it was (count=%d)", tt.httpMethod, mockDB.pullCount)
+				if mockDB.getPullCount() > 0 {
+					t.Errorf("Expected pull count NOT to be incremented for %s request, but it was (count=%d)", tt.httpMethod, mockDB.getPullCount())
 				}
 			}
 		})

pkg/appview/storage/profile_test.go

@@ -219,6 +219,7 @@ func TestGetProfile(t *testing.T) {
 			// Clear migration locks before each test
 			migrationLocks = sync.Map{}
 
+			var mu sync.Mutex
 			putRecordCalled := false
 			var migrationRequest map[string]any
 
@@ -232,8 +233,10 @@ func TestGetProfile(t *testing.T) {
 
 				// PutRecord (migration)
 				if r.Method == "POST" && strings.Contains(r.URL.Path, "putRecord") {
+					mu.Lock()
 					putRecordCalled = true
 					json.NewDecoder(r.Body).Decode(&migrationRequest)
+					mu.Unlock()
 					w.WriteHeader(http.StatusOK)
 					w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.sailor.profile/self","cid":"bafytest"}`))
 					return
@@ -270,12 +273,17 @@ func TestGetProfile(t *testing.T) {
 					// Give goroutine time to execute
 					time.Sleep(50 * time.Millisecond)
 
-					if !putRecordCalled {
+					mu.Lock()
+					called := putRecordCalled
+					request := migrationRequest
+					mu.Unlock()
+
+					if !called {
 						t.Error("Expected migration PutRecord to be called")
 					}
 
-					if migrationRequest != nil {
-						recordData := migrationRequest["record"].(map[string]any)
+					if request != nil {
+						recordData := request["record"].(map[string]any)
 						migratedHold := recordData["defaultHold"]
 						if migratedHold != tt.expectedHoldDID {
 							t.Errorf("Migrated defaultHold = %v, want %v", migratedHold, tt.expectedHoldDID)

pkg/appview/storage/routing_repository.go

@@ -7,6 +7,7 @@ package storage
 import (
 	"context"
 	"log/slog"
+	"sync"
 	"time"
 
 	"github.com/distribution/distribution/v3"
@@ -17,6 +18,7 @@ import (
 type RoutingRepository struct {
 	distribution.Repository
 	Ctx           *RegistryContext // All context and services (exported for token updates)
+	mu            sync.Mutex       // Protects manifestStore and blobStore
 	manifestStore *ManifestStore   // Cached manifest store instance
 	blobStore     *ProxyBlobStore  // Cached blob store instance
 }
@@ -31,35 +33,47 @@ func NewRoutingRepository(baseRepo distribution.Repository, ctx *RegistryContext
 
 // Manifests returns the ATProto-backed manifest service
 func (r *RoutingRepository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) {
+	r.mu.Lock()
 	// Create or return cached manifest store
 	if r.manifestStore == nil {
 		// Ensure blob store is created first (needed for label extraction during push)
+		// Release lock while calling Blobs to avoid deadlock
+		r.mu.Unlock()
 		blobStore := r.Blobs(ctx)
+		r.mu.Lock()
 
-		r.manifestStore = NewManifestStore(r.Ctx, blobStore)
+		// Double-check after reacquiring lock (another goroutine might have set it)
+		if r.manifestStore == nil {
+			r.manifestStore = NewManifestStore(r.Ctx, blobStore)
+		}
 	}
+	manifestStore := r.manifestStore
+	r.mu.Unlock()
 
 	// After any manifest operation, cache the hold DID for blob fetches
 	// We use a goroutine to avoid blocking, and check after a short delay to allow the operation to complete
 	go func() {
 		time.Sleep(100 * time.Millisecond) // Brief delay to let manifest fetch complete
-		if holdDID := r.manifestStore.GetLastFetchedHoldDID(); holdDID != "" {
+		if holdDID := manifestStore.GetLastFetchedHoldDID(); holdDID != "" {
 			// Cache for 10 minutes - should cover typical pull operations
 			GetGlobalHoldCache().Set(r.Ctx.DID, r.Ctx.Repository, holdDID, 10*time.Minute)
 			slog.Debug("Cached hold DID", "component", "storage/routing", "did", r.Ctx.DID, "repo", r.Ctx.Repository, "hold", holdDID)
 		}
 	}()
 
-	return r.manifestStore, nil
+	return manifestStore, nil
 }
 
 // Blobs returns a proxy blob store that routes to external hold service
 // The registry (AppView) NEVER stores blobs locally - all blobs go through hold service
 func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore {
+	r.mu.Lock()
 	// Return cached blob store if available
 	if r.blobStore != nil {
+		blobStore := r.blobStore
+		r.mu.Unlock()
 		slog.Debug("Returning cached blob store", "component", "storage/blobs", "did", r.Ctx.DID, "repo", r.Ctx.Repository)
-		return r.blobStore
+		return blobStore
 	}
 
 	// For pull operations, check if we have a cached hold DID from a recent manifest fetch
@@ -85,7 +99,9 @@ func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore {
 
 	// Create and cache proxy blob store
 	r.blobStore = NewProxyBlobStore(r.Ctx)
-	return r.blobStore
+	blobStore := r.blobStore
+	r.mu.Unlock()
+	return blobStore
 }
 
 // Tags returns the tag service

pkg/appview/templates/components/head.html

@@ -12,11 +12,11 @@
     <!-- Stylesheets -->
     <link rel="stylesheet" href="/css/style.css">
 
-    <!-- HTMX -->
-    <script src="https://unpkg.com/htmx.org@2.0.8/dist/htmx.min.js"></script>
+    <!-- HTMX (vendored) -->
+    <script src="/js/htmx.min.js"></script>
 
-    <!-- Lucide Icons -->
-    <script src="https://unpkg.com/lucide@latest"></script>
+    <!-- Lucide Icons (vendored) -->
+    <script src="/js/lucide.min.js"></script>
 
     <!-- App Scripts -->
     <script src="/js/app.js"></script>

pkg/appview/ui.go

@@ -12,6 +12,9 @@ import (
 	"atcr.io/pkg/appview/licenses"
 )
 
+//go:generate curl -fsSL -o static/js/htmx.min.js https://unpkg.com/htmx.org@2.0.8/dist/htmx.min.js
+//go:generate curl -fsSL -o static/js/lucide.min.js https://unpkg.com/lucide@latest/dist/umd/lucide.min.js
+
 //go:embed templates/**/*.html
 var templatesFS embed.FS
 

pkg/atproto/generate.go

@@ -35,6 +35,4 @@ func main() {
 		fmt.Printf("Failed to generate CBOR encoders: %v\n", err)
 		os.Exit(1)
 	}
-
-	fmt.Println("Generated CBOR encoders in pkg/atproto/cbor_gen.go")
 }