From 2a795ed5cd2b7fd4920e808c5b1289fa0b9ec374 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Fri, 24 Oct 2025 10:24:05 -0500 Subject: [PATCH] fix readmes not updating on repository page. attempt to fix not being able to send manifest to hold --- cmd/appview/serve.go | 4 ++ pkg/appview/middleware/registry.go | 10 ++++ pkg/appview/static/css/style.css | 6 ++ pkg/appview/storage/context.go | 15 ++++- pkg/appview/storage/manifest_store.go | 70 +++++++++++++++++----- pkg/appview/storage/manifest_store_test.go | 16 ++--- pkg/appview/storage/routing_repository.go | 59 +----------------- 7 files changed, 95 insertions(+), 85 deletions(-) diff --git a/cmd/appview/serve.go b/cmd/appview/serve.go index e8edd00..fd3d073 100644 --- a/cmd/appview/serve.go +++ b/cmd/appview/serve.go @@ -201,6 +201,10 @@ func serveRegistry(cmd *cobra.Command, args []string) error { middleware.SetGlobalAuthorizer(holdAuthorizer) fmt.Println("Hold authorizer initialized with database caching") + // Set global readme cache for middleware + middleware.SetGlobalReadmeCache(readmeCache) + fmt.Println("README cache initialized for manifest push refresh") + // Initialize UI routes with OAuth app, refresher, device store, health checker, and readme cache uiTemplates, uiRouter := initializeUIRoutes(uiDatabase, uiReadOnlyDB, uiSessionStore, oauthApp, refresher, baseURL, deviceStore, defaultHoldDID, healthChecker, readmeCache) diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index ab07133..716ae09 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -36,6 +36,7 @@ var ( globalRefresher *oauth.Refresher globalDatabase storage.DatabaseMetrics globalAuthorizer auth.HoldAuthorizer + globalReadmeCache storage.ReadmeCache ) // SetGlobalRefresher sets the OAuth refresher instance during initialization @@ -56,6 +57,12 @@ func SetGlobalAuthorizer(authorizer auth.HoldAuthorizer) { globalAuthorizer = authorizer } +// SetGlobalReadmeCache sets the readme cache instance during initialization +// Must be called before the registry starts serving requests +func SetGlobalReadmeCache(readmeCache storage.ReadmeCache) { + globalReadmeCache = readmeCache +} + func init() { // Register the name resolution middleware registrymw.Register("atproto-resolver", initATProtoResolver) @@ -71,6 +78,7 @@ type NamespaceResolver struct { refresher *oauth.Refresher // OAuth session manager (copied from global on init) database storage.DatabaseMetrics // Metrics database (copied from global on init) authorizer auth.HoldAuthorizer // Hold authorization (copied from global on init) + readmeCache storage.ReadmeCache // README cache (copied from global on init) } // initATProtoResolver initializes the name resolution middleware @@ -101,6 +109,7 @@ func initATProtoResolver(ctx context.Context, ns distribution.Namespace, _ drive refresher: globalRefresher, database: globalDatabase, authorizer: globalAuthorizer, + readmeCache: globalReadmeCache, }, nil } @@ -321,6 +330,7 @@ func (nr *NamespaceResolver) Repository(ctx context.Context, name reference.Name Database: nr.database, Authorizer: nr.authorizer, Refresher: nr.refresher, + ReadmeCache: nr.readmeCache, } routingRepo := storage.NewRoutingRepository(repo, registryCtx) diff --git a/pkg/appview/static/css/style.css b/pkg/appview/static/css/style.css index 2c02116..2179c3e 100644 --- a/pkg/appview/static/css/style.css +++ b/pkg/appview/static/css/style.css @@ -1707,6 +1707,8 @@ a.license-badge:hover { border: 1px solid var(--border); border-radius: 8px; padding: 2rem; + min-width: 0; + box-sizing: border-box; } .readme-section h2 { @@ -1717,6 +1719,8 @@ a.license-badge:hover { .readme-content { overflow-wrap: break-word; + max-width: 100%; + box-sizing: border-box; } .repo-sidebar { @@ -1814,6 +1818,8 @@ a.license-badge:hover { border-radius: 6px; overflow-x: auto; margin-bottom: 1rem; + max-width: 100%; + box-sizing: border-box; } .markdown-body pre code { diff --git a/pkg/appview/storage/context.go b/pkg/appview/storage/context.go index b4dc7c0..14fc138 100644 --- a/pkg/appview/storage/context.go +++ b/pkg/appview/storage/context.go @@ -1,6 +1,8 @@ package storage import ( + "context" + "atcr.io/pkg/atproto" "atcr.io/pkg/auth" "atcr.io/pkg/auth/oauth" @@ -12,6 +14,12 @@ type DatabaseMetrics interface { IncrementPushCount(did, repository string) error } +// ReadmeCache interface for README content caching +type ReadmeCache interface { + Get(ctx context.Context, url string) (string, error) + Invalidate(url string) error +} + // RegistryContext bundles all the context needed for registry operations // This includes both per-request data (DID, hold) and shared services type RegistryContext struct { @@ -25,7 +33,8 @@ type RegistryContext struct { ATProtoClient *atproto.Client // Authenticated ATProto client for this user // 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 + ReadmeCache ReadmeCache // README content cache } diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go index 420b718..2c7cc6b 100644 --- a/pkg/appview/storage/manifest_store.go +++ b/pkg/appview/storage/manifest_store.go @@ -10,31 +10,25 @@ import ( "maps" "net/http" "strings" + "time" "atcr.io/pkg/atproto" "github.com/distribution/distribution/v3" "github.com/opencontainers/go-digest" ) -// HoldNotifier interface for notifying holds about manifest uploads -type HoldNotifier interface { - GetServiceToken(ctx context.Context, userDID, audienceDID string) (string, error) -} - // ManifestStore implements distribution.ManifestService // It stores manifests in ATProto as records type ManifestStore struct { ctx *RegistryContext // Context with user/hold info - notifier HoldNotifier // OAuth refresher for getting service tokens lastFetchedHoldDID string // Hold DID from most recently fetched manifest (for pull) blobStore distribution.BlobStore // Blob store for fetching config during push } // NewManifestStore creates a new ATProto-backed manifest store -func NewManifestStore(ctx *RegistryContext, notifier HoldNotifier, blobStore distribution.BlobStore) *ManifestStore { +func NewManifestStore(ctx *RegistryContext, blobStore distribution.BlobStore) *ManifestStore { return &ManifestStore{ ctx: ctx, - notifier: notifier, blobStore: blobStore, } } @@ -195,7 +189,7 @@ func (s *ManifestStore) Put(ctx context.Context, manifest distribution.Manifest, // Notify hold about manifest upload (for layer tracking and Bluesky posts) // Do this asynchronously to avoid blocking the push - if tag != "" && s.notifier != nil && s.ctx.Handle != "" { + if tag != "" && s.ctx.ServiceToken != "" && s.ctx.Handle != "" { go func() { if err := s.notifyHoldAboutManifest(context.Background(), manifestRecord, tag, dgst.String()); err != nil { fmt.Printf("WARNING: Failed to notify hold about manifest: %v\n", err) @@ -203,6 +197,12 @@ func (s *ManifestStore) Put(ctx context.Context, manifest distribution.Manifest, }() } + // Refresh README cache asynchronously if manifest has io.atcr.readme annotation + // This ensures fresh README content is available on repository pages + go func() { + s.refreshReadmeCache(context.Background(), manifestRecord) + }() + return dgst, nil } @@ -287,8 +287,8 @@ func resolveDIDToHTTPSEndpoint(did string) (string, error) { // notifyHoldAboutManifest notifies the hold service about a manifest upload // This enables the hold to create layer records and Bluesky posts func (s *ManifestStore) notifyHoldAboutManifest(ctx context.Context, manifestRecord *atproto.ManifestRecord, tag, manifestDigest string) error { - // Skip if no notifier configured - if s.notifier == nil { + // Skip if no service token configured (e.g., anonymous pulls) + if s.ctx.ServiceToken == "" { return nil } @@ -299,11 +299,8 @@ func (s *ManifestStore) notifyHoldAboutManifest(ctx context.Context, manifestRec return fmt.Errorf("failed to resolve hold DID %s: %w", s.ctx.HoldDID, err) } - // Get service token from user's PDS for hold authentication - serviceToken, err := s.notifier.GetServiceToken(ctx, s.ctx.DID, s.ctx.HoldDID) - if err != nil { - return fmt.Errorf("failed to get service token: %w", err) - } + // Use service token from middleware (already cached and validated) + serviceToken := s.ctx.ServiceToken // Build notification request notifyReq := map[string]any{ @@ -370,3 +367,44 @@ func (s *ManifestStore) notifyHoldAboutManifest(ctx context.Context, manifestRec return nil } + +// refreshReadmeCache refreshes the README cache for this manifest if it has io.atcr.readme annotation +// This should be called asynchronously after manifest push to keep README content fresh +func (s *ManifestStore) refreshReadmeCache(ctx context.Context, manifestRecord *atproto.ManifestRecord) { + // Skip if no README cache configured + if s.ctx.ReadmeCache == nil { + return + } + + // Skip if no annotations or no README URL + if manifestRecord.Annotations == nil { + return + } + + readmeURL, ok := manifestRecord.Annotations["io.atcr.readme"] + if !ok || readmeURL == "" { + return + } + + fmt.Printf("INFO: Refreshing README cache for %s/%s from %s\n", s.ctx.DID, s.ctx.Repository, readmeURL) + + // Invalidate the cached entry first + if err := s.ctx.ReadmeCache.Invalidate(readmeURL); err != nil { + fmt.Printf("WARNING: Failed to invalidate README cache for %s: %v\n", readmeURL, err) + // Continue anyway - Get() will still fetch fresh content + } + + // Fetch fresh content to populate cache + // Use context with timeout to avoid hanging on slow/dead URLs + ctxWithTimeout, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + _, err := s.ctx.ReadmeCache.Get(ctxWithTimeout, readmeURL) + if err != nil { + fmt.Printf("WARNING: Failed to refresh README cache for %s: %v\n", readmeURL, err) + // Not a critical error - cache will be refreshed on next page view + return + } + + fmt.Printf("INFO: README cache refreshed successfully for %s\n", readmeURL) +} diff --git a/pkg/appview/storage/manifest_store_test.go b/pkg/appview/storage/manifest_store_test.go index 721337c..39a961e 100644 --- a/pkg/appview/storage/manifest_store_test.go +++ b/pkg/appview/storage/manifest_store_test.go @@ -141,7 +141,7 @@ func TestNewManifestStore(t *testing.T) { db := &mockDatabaseMetrics{} ctx := mockRegistryContext(client, "myapp", "did:web:hold.example.com", "did:plc:alice123", "alice.test", db) - store := NewManifestStore(ctx, nil, blobStore) + store := NewManifestStore(ctx, blobStore) if store.ctx.Repository != "myapp" { t.Errorf("repository = %v, want myapp", store.ctx.Repository) @@ -189,7 +189,7 @@ func TestManifestStore_GetLastFetchedHoldDID(t *testing.T) { t.Run(tt.name, func(t *testing.T) { client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "", "did:plc:test123", "test.handle", nil) - store := NewManifestStore(ctx, nil, nil) + store := NewManifestStore(ctx, nil) // Simulate what happens in Get() when parsing a manifest record var manifestRecord atproto.ManifestRecord @@ -264,7 +264,7 @@ func TestExtractConfigLabels(t *testing.T) { // Create manifest store client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "", "did:plc:test123", "test.handle", nil) - store := NewManifestStore(ctx, nil, blobStore) + store := NewManifestStore(ctx, blobStore) // Extract labels labels, err := store.extractConfigLabels(context.Background(), configDigest.String()) @@ -304,7 +304,7 @@ func TestExtractConfigLabels_NoLabels(t *testing.T) { client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "", "did:plc:test123", "test.handle", nil) - store := NewManifestStore(ctx, nil, blobStore) + store := NewManifestStore(ctx, blobStore) labels, err := store.extractConfigLabels(context.Background(), configDigest.String()) if err != nil { @@ -322,7 +322,7 @@ func TestExtractConfigLabels_InvalidDigest(t *testing.T) { blobStore := newMockBlobStore() client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "", "did:plc:test123", "test.handle", nil) - store := NewManifestStore(ctx, nil, blobStore) + store := NewManifestStore(ctx, blobStore) _, err := store.extractConfigLabels(context.Background(), "invalid-digest") if err == nil { @@ -341,7 +341,7 @@ func TestExtractConfigLabels_InvalidJSON(t *testing.T) { client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "", "did:plc:test123", "test.handle", nil) - store := NewManifestStore(ctx, nil, blobStore) + store := NewManifestStore(ctx, blobStore) _, err := store.extractConfigLabels(context.Background(), configDigest.String()) if err == nil { @@ -354,7 +354,7 @@ func TestManifestStore_WithMetrics(t *testing.T) { db := &mockDatabaseMetrics{} client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "did:web:hold.example.com", "did:plc:alice123", "alice.test", db) - store := NewManifestStore(ctx, nil, nil) + store := NewManifestStore(ctx, nil) if store.ctx.Database != db { t.Error("ManifestStore should store database reference") @@ -368,7 +368,7 @@ func TestManifestStore_WithMetrics(t *testing.T) { func TestManifestStore_WithoutMetrics(t *testing.T) { client := atproto.NewClient("https://pds.example.com", "did:plc:test123", "token") ctx := mockRegistryContext(client, "myapp", "did:web:hold.example.com", "did:plc:alice123", "alice.test", nil) - store := NewManifestStore(ctx, nil, nil) + store := NewManifestStore(ctx, nil) if store.ctx.Database != nil { t.Error("ManifestStore should accept nil database") diff --git a/pkg/appview/storage/routing_repository.go b/pkg/appview/storage/routing_repository.go index 978b37f..c44a3f3 100644 --- a/pkg/appview/storage/routing_repository.go +++ b/pkg/appview/storage/routing_repository.go @@ -6,13 +6,9 @@ package storage import ( "context" - "encoding/json" "fmt" - "io" - "net/http" "time" - "atcr.io/pkg/auth/oauth" "github.com/distribution/distribution/v3" ) @@ -25,50 +21,6 @@ type RoutingRepository struct { blobStore *ProxyBlobStore // Cached blob store instance } -// refresherAdapter adapts the oauth.Refresher to implement atproto.HoldNotifier -type refresherAdapter struct { - refresher *oauth.Refresher - pdsEndpoint string -} - -// GetServiceToken implements atproto.HoldNotifier -func (r *refresherAdapter) GetServiceToken(ctx context.Context, userDID, audienceDID string) (string, error) { - // Get OAuth session for the user - session, err := r.refresher.GetSession(ctx, userDID) - if err != nil { - return "", fmt.Errorf("failed to get OAuth session: %w", err) - } - - // Build service auth URL - serviceAuthURL := fmt.Sprintf("%s/xrpc/com.atproto.server.getServiceAuth?aud=%s", r.pdsEndpoint, audienceDID) - - req, err := http.NewRequestWithContext(ctx, "GET", serviceAuthURL, nil) - if err != nil { - return "", fmt.Errorf("failed to create request: %w", err) - } - - // Use session's DoWithAuth to handle OAuth authentication automatically - resp, err := session.DoWithAuth(session.Client, req, "com.atproto.server.getServiceAuth") - if err != nil { - return "", fmt.Errorf("failed to request service token: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return "", fmt.Errorf("PDS returned status %d: %s", resp.StatusCode, body) - } - - var result struct { - Token string `json:"token"` - } - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return "", fmt.Errorf("failed to decode response: %w", err) - } - - return result.Token, nil -} - // NewRoutingRepository creates a new routing repository func NewRoutingRepository(baseRepo distribution.Repository, ctx *RegistryContext) *RoutingRepository { return &RoutingRepository{ @@ -84,16 +36,7 @@ func (r *RoutingRepository) Manifests(ctx context.Context, options ...distributi // Ensure blob store is created first (needed for label extraction during push) blobStore := r.Blobs(ctx) - // Wrap the Refresher in an adapter to implement HoldNotifier - var notifier HoldNotifier - if r.Ctx.Refresher != nil { - notifier = &refresherAdapter{ - refresher: r.Ctx.Refresher, - pdsEndpoint: r.Ctx.PDSEndpoint, - } - } - - r.manifestStore = NewManifestStore(r.Ctx, notifier, blobStore) + r.manifestStore = NewManifestStore(r.Ctx, blobStore) } // After any manifest operation, cache the hold DID for blob fetches