better handling for io.atcr.repo.page

lexicons/io/atcr/repo/page.json

@@ -24,7 +24,7 @@
             "type": "blob",
             "description": "Repository avatar/icon image.",
             "accept": ["image/png", "image/jpeg", "image/webp"],
-            "maxSize": 1000000
+            "maxSize": 3000000
           },
           "createdAt": {
             "type": "string",

pkg/appview/middleware/registry.go

@@ -2,7 +2,6 @@ package middleware
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -16,6 +15,7 @@ import (
 	"github.com/distribution/distribution/v3/registry/storage/driver"
 	"github.com/distribution/reference"
 
+	"atcr.io/pkg/appview/readme"
 	"atcr.io/pkg/appview/storage"
 	"atcr.io/pkg/atproto"
 	"atcr.io/pkg/auth"
@@ -208,6 +208,7 @@ type NamespaceResolver struct {
 	database        storage.DatabaseMetrics // Metrics database (copied from global on init)
 	authorizer      auth.HoldAuthorizer     // Hold authorization (copied from global on init)
 	validationCache *validationCache        // Request-level service token cache
+	readmeFetcher   *readme.Fetcher         // README fetcher for repo pages
 }
 
 // initATProtoResolver initializes the name resolution middleware
@@ -242,6 +243,7 @@ func initATProtoResolver(ctx context.Context, ns distribution.Namespace, _ drive
 		database:        globalDatabase,
 		authorizer:      globalAuthorizer,
 		validationCache: newValidationCache(),
+		readmeFetcher:   readme.NewFetcher(),
 	}, nil
 }
 
@@ -458,6 +460,7 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name
 		Database:      nr.database,
 		Authorizer:    nr.authorizer,
 		Refresher:     nr.refresher,
+		ReadmeFetcher: nr.readmeFetcher,
 	}
 
 	return storage.NewRoutingRepository(repo, registryCtx), nil
@@ -481,8 +484,7 @@ func (nr *NamespaceResolver) BlobStatter() distribution.BlobStatter {
 // findHoldDID determines which hold DID to use for blob storage
 // Priority order:
 // 1. User's sailor profile defaultHold (if set)
-// 2. User's own hold record (io.atcr.hold)
-// 3. AppView's default hold DID
+// 2. AppView's default hold DID
 // Returns a hold DID (e.g., "did:web:hold01.atcr.io"), or empty string if none configured
 func (nr *NamespaceResolver) findHoldDID(ctx context.Context, did, pdsEndpoint string) string {
 	// Create ATProto client (without auth - reading public records)
@@ -508,28 +510,7 @@ func (nr *NamespaceResolver) findHoldDID(ctx context.Context, did, pdsEndpoint s
 		return profile.DefaultHold
 	}
 
-	// Profile doesn't exist or defaultHold is null/empty
-	// Check for user's own hold records
-	records, err := client.ListRecords(ctx, atproto.HoldCollection, 10)
-	if err != nil {
-		// Failed to query holds, use default
-		return nr.defaultHoldDID
-	}
-
-	// Find the first hold record
-	for _, record := range records {
-		var holdRecord atproto.HoldRecord
-		if err := json.Unmarshal(record.Value, &holdRecord); err != nil {
-			continue
-		}
-
-		// Return the endpoint from the first hold (normalize to DID if URL)
-		if holdRecord.Endpoint != "" {
-			return atproto.ResolveHoldDIDFromURL(holdRecord.Endpoint)
-		}
-	}
-
-	// No profile defaultHold and no own hold records - use AppView default
+	// No profile defaultHold - use AppView default
 	return nr.defaultHoldDID
 }
 

pkg/appview/middleware/registry_test.go

@@ -199,45 +199,6 @@ func TestFindHoldDID_SailorProfile(t *testing.T) {
 	assert.Equal(t, "did:web:user.hold.io", holdDID, "should use sailor profile's defaultHold")
 }
 
-// TestFindHoldDID_LegacyHoldRecords tests legacy hold record discovery
-func TestFindHoldDID_LegacyHoldRecords(t *testing.T) {
-	// Start a mock PDS server that returns hold records
-	mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path == "/xrpc/com.atproto.repo.getRecord" {
-			// Profile not found
-			w.WriteHeader(http.StatusNotFound)
-			return
-		}
-		if r.URL.Path == "/xrpc/com.atproto.repo.listRecords" {
-			// Return hold record
-			holdRecord := atproto.NewHoldRecord("https://legacy.hold.io", "alice", true)
-			recordJSON, _ := json.Marshal(holdRecord)
-			w.Header().Set("Content-Type", "application/json")
-			json.NewEncoder(w).Encode(map[string]any{
-				"records": []any{
-					map[string]any{
-						"uri":   "at://did:plc:test123/io.atcr.hold/abc123",
-						"value": json.RawMessage(recordJSON),
-					},
-				},
-			})
-			return
-		}
-		w.WriteHeader(http.StatusNotFound)
-	}))
-	defer mockPDS.Close()
-
-	resolver := &NamespaceResolver{
-		defaultHoldDID: "did:web:default.atcr.io",
-	}
-
-	ctx := context.Background()
-	holdDID := resolver.findHoldDID(ctx, "did:plc:test123", mockPDS.URL)
-
-	// Legacy URL should be converted to DID
-	assert.Equal(t, "did:web:legacy.hold.io", holdDID, "should use legacy hold record and convert to DID")
-}
-
 // TestFindHoldDID_Priority tests the priority order
 func TestFindHoldDID_Priority(t *testing.T) {
 	// Start a mock PDS server that returns both profile and hold records
@@ -251,21 +212,6 @@ func TestFindHoldDID_Priority(t *testing.T) {
 			})
 			return
 		}
-		if r.URL.Path == "/xrpc/com.atproto.repo.listRecords" {
-			// Return hold record (should be ignored since profile exists)
-			holdRecord := atproto.NewHoldRecord("https://legacy.hold.io", "alice", true)
-			recordJSON, _ := json.Marshal(holdRecord)
-			w.Header().Set("Content-Type", "application/json")
-			json.NewEncoder(w).Encode(map[string]any{
-				"records": []any{
-					map[string]any{
-						"uri":   "at://did:plc:test123/io.atcr.hold/abc123",
-						"value": json.RawMessage(recordJSON),
-					},
-				},
-			})
-			return
-		}
 		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer mockPDS.Close()

pkg/appview/storage/context.go

@@ -1,6 +1,7 @@
 package storage
 
 import (
+	"atcr.io/pkg/appview/readme"
 	"atcr.io/pkg/atproto"
 	"atcr.io/pkg/auth"
 	"atcr.io/pkg/auth/oauth"
@@ -27,7 +28,8 @@ type RegistryContext struct {
 	AuthMethod    string          // Auth method used ("oauth" or "app_password")
 
 	// Shared services (same for all requests)
-	Database   DatabaseMetrics     // Metrics tracking database
-	Authorizer auth.HoldAuthorizer // Hold access authorization
-	Refresher  *oauth.Refresher    // OAuth session manager
+	Database      DatabaseMetrics     // Metrics tracking database
+	Authorizer    auth.HoldAuthorizer // Hold access authorization
+	Refresher     *oauth.Refresher    // OAuth session manager
+	ReadmeFetcher *readme.Fetcher     // README fetcher for repo pages
 }

pkg/appview/storage/manifest_store.go

@@ -12,7 +12,9 @@ import (
 	"net/http"
 	"strings"
 	"sync"
+	"time"
 
+	"atcr.io/pkg/appview/readme"
 	"atcr.io/pkg/atproto"
 	"github.com/distribution/distribution/v3"
 	"github.com/opencontainers/go-digest"
@@ -432,13 +434,6 @@ func (s *ManifestStore) ensureRepoPage(ctx context.Context, manifestRecord *atpr
 		return
 	}
 
-	// Check for relevant annotations that we can use for repo page
-	description := manifestRecord.Annotations["org.opencontainers.image.description"]
-	if description == "" {
-		// No description annotation - nothing to create
-		return
-	}
-
 	// Check if repo page already exists (don't overwrite user's custom content)
 	rkey := s.ctx.Repository
 	_, err := s.ctx.ATProtoClient.GetRecord(ctx, atproto.RepoPageCollection, rkey)
@@ -454,12 +449,31 @@ func (s *ManifestStore) ensureRepoPage(ctx context.Context, manifestRecord *atpr
 		return
 	}
 
-	// Create new repo page record from manifest annotations
-	// Note: Avatar is not extracted from annotations here - that's handled separately
-	// (would require uploading a blob if annotation contains a URL)
-	repoPage := atproto.NewRepoPageRecord(s.ctx.Repository, description, nil)
+	// Try to fetch README content from external sources
+	// Priority: io.atcr.readme annotation > derived from org.opencontainers.image.source > org.opencontainers.image.description
+	description := s.fetchReadmeContent(ctx, manifestRecord.Annotations)
 
-	slog.Info("Creating repo page from manifest annotations", "did", s.ctx.DID, "repository", s.ctx.Repository)
+	// If no README content could be fetched, fall back to description annotation
+	if description == "" {
+		description = manifestRecord.Annotations["org.opencontainers.image.description"]
+	}
+
+	// Try to fetch and upload icon from io.atcr.icon annotation
+	var avatarRef *atproto.ATProtoBlobRef
+	if iconURL := manifestRecord.Annotations["io.atcr.icon"]; iconURL != "" {
+		avatarRef = s.fetchAndUploadIcon(ctx, iconURL)
+	}
+
+	// If no description and no icon, nothing to create
+	if description == "" && avatarRef == nil {
+		slog.Debug("No README, description, or icon found for repo page", "did", s.ctx.DID, "repository", s.ctx.Repository)
+		return
+	}
+
+	// Create new repo page record with description and optional avatar
+	repoPage := atproto.NewRepoPageRecord(s.ctx.Repository, description, avatarRef)
+
+	slog.Info("Creating repo page from manifest annotations", "did", s.ctx.DID, "repository", s.ctx.Repository, "descriptionLength", len(description), "hasAvatar", avatarRef != nil)
 
 	_, err = s.ctx.ATProtoClient.PutRecord(ctx, atproto.RepoPageCollection, rkey, repoPage)
 	if err != nil {
@@ -469,3 +483,195 @@ func (s *ManifestStore) ensureRepoPage(ctx context.Context, manifestRecord *atpr
 
 	slog.Info("Repo page created successfully", "did", s.ctx.DID, "repository", s.ctx.Repository)
 }
+
+// fetchReadmeContent attempts to fetch README content from external sources
+// Priority: io.atcr.readme annotation > derived from org.opencontainers.image.source
+// Returns the raw markdown content, or empty string if not available
+func (s *ManifestStore) fetchReadmeContent(ctx context.Context, annotations map[string]string) string {
+	if s.ctx.ReadmeFetcher == nil {
+		return ""
+	}
+
+	// Create a context with timeout for README fetching (don't block push too long)
+	fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	defer cancel()
+
+	// Priority 1: Direct README URL from io.atcr.readme annotation
+	if readmeURL := annotations["io.atcr.readme"]; readmeURL != "" {
+		content, err := s.fetchRawReadme(fetchCtx, readmeURL)
+		if err != nil {
+			slog.Debug("Failed to fetch README from io.atcr.readme annotation", "url", readmeURL, "error", err)
+		} else if content != "" {
+			slog.Info("Fetched README from io.atcr.readme annotation", "url", readmeURL, "length", len(content))
+			return content
+		}
+	}
+
+	// Priority 2: Derive README URL from org.opencontainers.image.source
+	if sourceURL := annotations["org.opencontainers.image.source"]; sourceURL != "" {
+		// Try main branch first, then master
+		for _, branch := range []string{"main", "master"} {
+			readmeURL := readme.DeriveReadmeURL(sourceURL, branch)
+			if readmeURL == "" {
+				continue
+			}
+
+			content, err := s.fetchRawReadme(fetchCtx, readmeURL)
+			if err != nil {
+				// Only log non-404 errors (404 is expected when trying main vs master)
+				if !readme.Is404(err) {
+					slog.Debug("Failed to fetch README from source URL", "url", readmeURL, "branch", branch, "error", err)
+				}
+				continue
+			}
+
+			if content != "" {
+				slog.Info("Fetched README from source URL", "sourceURL", sourceURL, "branch", branch, "length", len(content))
+				return content
+			}
+		}
+	}
+
+	return ""
+}
+
+// fetchRawReadme fetches raw markdown content from a URL
+// Returns the raw markdown (not rendered HTML) for storage in the repo page record
+func (s *ManifestStore) fetchRawReadme(ctx context.Context, readmeURL string) (string, error) {
+	// Use a simple HTTP client to fetch raw content
+	// We want raw markdown, not rendered HTML (the Fetcher renders to HTML)
+	req, err := http.NewRequestWithContext(ctx, "GET", readmeURL, nil)
+	if err != nil {
+		return "", fmt.Errorf("failed to create request: %w", err)
+	}
+
+	req.Header.Set("User-Agent", "ATCR-README-Fetcher/1.0")
+
+	client := &http.Client{
+		Timeout: 10 * time.Second,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if len(via) >= 5 {
+				return fmt.Errorf("too many redirects")
+			}
+			return nil
+		},
+	}
+
+	resp, err := client.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("failed to fetch URL: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("unexpected status code: %d", resp.StatusCode)
+	}
+
+	// Limit content size to 100KB (repo page description has 100KB limit in lexicon)
+	limitedReader := io.LimitReader(resp.Body, 100*1024)
+	content, err := io.ReadAll(limitedReader)
+	if err != nil {
+		return "", fmt.Errorf("failed to read response body: %w", err)
+	}
+
+	return string(content), nil
+}
+
+// fetchAndUploadIcon fetches an image from a URL and uploads it as a blob to the user's PDS
+// Returns the blob reference for use in the repo page record, or nil on error
+func (s *ManifestStore) fetchAndUploadIcon(ctx context.Context, iconURL string) *atproto.ATProtoBlobRef {
+	// Create a context with timeout for icon fetching
+	fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	defer cancel()
+
+	// Fetch the icon
+	req, err := http.NewRequestWithContext(fetchCtx, "GET", iconURL, nil)
+	if err != nil {
+		slog.Debug("Failed to create icon request", "url", iconURL, "error", err)
+		return nil
+	}
+
+	req.Header.Set("User-Agent", "ATCR-Icon-Fetcher/1.0")
+
+	client := &http.Client{
+		Timeout: 10 * time.Second,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if len(via) >= 5 {
+				return fmt.Errorf("too many redirects")
+			}
+			return nil
+		},
+	}
+
+	resp, err := client.Do(req)
+	if err != nil {
+		slog.Debug("Failed to fetch icon", "url", iconURL, "error", err)
+		return nil
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		slog.Debug("Icon fetch returned non-OK status", "url", iconURL, "status", resp.StatusCode)
+		return nil
+	}
+
+	// Validate content type - only allow images
+	contentType := resp.Header.Get("Content-Type")
+	mimeType := detectImageMimeType(contentType, iconURL)
+	if mimeType == "" {
+		slog.Debug("Icon has unsupported content type", "url", iconURL, "contentType", contentType)
+		return nil
+	}
+
+	// Limit icon size to 3MB (matching lexicon maxSize)
+	limitedReader := io.LimitReader(resp.Body, 3*1024*1024)
+	iconData, err := io.ReadAll(limitedReader)
+	if err != nil {
+		slog.Debug("Failed to read icon data", "url", iconURL, "error", err)
+		return nil
+	}
+
+	if len(iconData) == 0 {
+		slog.Debug("Icon data is empty", "url", iconURL)
+		return nil
+	}
+
+	// Upload the icon as a blob to the user's PDS
+	blobRef, err := s.ctx.ATProtoClient.UploadBlob(ctx, iconData, mimeType)
+	if err != nil {
+		slog.Warn("Failed to upload icon blob", "url", iconURL, "error", err)
+		return nil
+	}
+
+	slog.Info("Uploaded icon blob", "url", iconURL, "size", len(iconData), "mimeType", mimeType, "cid", blobRef.Ref.Link)
+	return blobRef
+}
+
+// detectImageMimeType determines the MIME type for an image
+// Uses Content-Type header first, then falls back to extension-based detection
+// Only allows types accepted by the lexicon: image/png, image/jpeg, image/webp
+func detectImageMimeType(contentType, url string) string {
+	// Check Content-Type header first
+	switch {
+	case strings.HasPrefix(contentType, "image/png"):
+		return "image/png"
+	case strings.HasPrefix(contentType, "image/jpeg"):
+		return "image/jpeg"
+	case strings.HasPrefix(contentType, "image/webp"):
+		return "image/webp"
+	}
+
+	// Fall back to URL extension detection
+	lowerURL := strings.ToLower(url)
+	switch {
+	case strings.HasSuffix(lowerURL, ".png"):
+		return "image/png"
+	case strings.HasSuffix(lowerURL, ".jpg"), strings.HasSuffix(lowerURL, ".jpeg"):
+		return "image/jpeg"
+	case strings.HasSuffix(lowerURL, ".webp"):
+		return "image/webp"
+	}
+
+	// Unknown or unsupported type - reject
+	return ""
+}

pkg/atproto/lexicon.go

@@ -18,9 +18,6 @@ const (
 	// TagCollection is the collection name for image tags
 	TagCollection = "io.atcr.tag"
 
-	// HoldCollection is the collection name for storage holds (BYOS)
-	HoldCollection = "io.atcr.hold"
-
 	// HoldCrewCollection is the collection name for hold crew (membership) - LEGACY BYOS model
 	// Stored in owner's PDS for BYOS holds
 	HoldCrewCollection = "io.atcr.hold.crew"
@@ -313,16 +310,6 @@ type HoldRecord struct {
 	CreatedAt time.Time `json:"createdAt"`
 }
 
-// NewHoldRecord creates a new hold record
-func NewHoldRecord(endpoint, owner string, public bool) *HoldRecord {
-	return &HoldRecord{
-		Type:      HoldCollection,
-		Endpoint:  endpoint,
-		Owner:     owner,
-		Public:    public,
-		CreatedAt: time.Now(),
-	}
-}
 
 // SailorProfileRecord represents a user's profile with registry preferences
 // Stored in the user's PDS to configure default hold and other settings

pkg/atproto/lexicon_test.go

@@ -452,56 +452,6 @@ func TestTagRecord_GetManifestDigest(t *testing.T) {
 	}
 }
 
-func TestNewHoldRecord(t *testing.T) {
-	tests := []struct {
-		name     string
-		endpoint string
-		owner    string
-		public   bool
-	}{
-		{
-			name:     "public hold",
-			endpoint: "https://hold1.example.com",
-			owner:    "did:plc:alice123",
-			public:   true,
-		},
-		{
-			name:     "private hold",
-			endpoint: "https://hold2.example.com",
-			owner:    "did:plc:bob456",
-			public:   false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			before := time.Now()
-			record := NewHoldRecord(tt.endpoint, tt.owner, tt.public)
-			after := time.Now()
-
-			if record.Type != HoldCollection {
-				t.Errorf("Type = %v, want %v", record.Type, HoldCollection)
-			}
-
-			if record.Endpoint != tt.endpoint {
-				t.Errorf("Endpoint = %v, want %v", record.Endpoint, tt.endpoint)
-			}
-
-			if record.Owner != tt.owner {
-				t.Errorf("Owner = %v, want %v", record.Owner, tt.owner)
-			}
-
-			if record.Public != tt.public {
-				t.Errorf("Public = %v, want %v", record.Public, tt.public)
-			}
-
-			if record.CreatedAt.Before(before) || record.CreatedAt.After(after) {
-				t.Errorf("CreatedAt = %v, want between %v and %v", record.CreatedAt, before, after)
-			}
-		})
-	}
-}
-
 func TestNewSailorProfileRecord(t *testing.T) {
 	tests := []struct {
 		name        string

pkg/auth/oauth/client.go

@@ -95,6 +95,7 @@ func GetDefaultScopes(did string) []string {
 		fmt.Sprintf("repo:%s", atproto.TagCollection),
 		fmt.Sprintf("repo:%s", atproto.StarCollection),
 		fmt.Sprintf("repo:%s", atproto.SailorProfileCollection),
+		fmt.Sprintf("repo:%s", atproto.RepoPageCollection),
 	)
 
 	return scopes