diff --git a/CLAUDE.md b/CLAUDE.md index 7153cda..781dc10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -96,9 +96,9 @@ client plain HTTP clients so it can reach a PDS on loopback. compiles them out and stays green while `make test` runs everything. The integration harness carries the same constraint. - The remaining local-dev behaviors (hold issuer-mismatch tolerance, skipped - relay crawl requests, backfill warning suppression, registry - fall-back-to-default-hold) read `atproto.TestModeBuild` too. There is no - runtime test-mode config key any more. + relay crawl requests, backfill warning suppression) read + `atproto.TestModeBuild` too. There is no runtime test-mode config key any + more. ## Architecture Overview diff --git a/pkg/appview/middleware/registry.go b/pkg/appview/middleware/registry.go index 7192e5e..56a19db 100644 --- a/pkg/appview/middleware/registry.go +++ b/pkg/appview/middleware/registry.go @@ -257,17 +257,16 @@ func init() { // NamespaceResolver wraps a namespace and resolves names type NamespaceResolver struct { distribution.Namespace - defaultHoldDID string // Default hold DID (e.g., "did:web:hold01.atcr.io") - baseURL string // Base URL for error messages (e.g., "https://atcr.io") - fallbackUnreachable bool // Fall back to the default hold when the user's hold is unreachable (testmode builds) - refresher *oauth.Refresher // OAuth session manager (copied from global on init) - database storage.HoldDIDLookup // Database for hold DID lookups (copied from global on init) - authorizer auth.HoldAuthorizer // Hold authorization (copied from global on init) - webhookDispatcher storage.PushWebhookDispatcher // Push webhook dispatcher (copied from global on init) - manifestRefChecker storage.ManifestReferenceChecker // Manifest reference checker (copied from global on init) - validationCache *validationCache // Request-level service token cache - readmeFetcher *readme.Fetcher // README fetcher for repo pages - userPrefs UserPrefsCache // Cached sailor profile preferences (copied from global on init) + defaultHoldDID string // Default hold DID (e.g., "did:web:hold01.atcr.io") + baseURL string // Base URL for error messages (e.g., "https://atcr.io") + refresher *oauth.Refresher // OAuth session manager (copied from global on init) + database storage.HoldDIDLookup // Database for hold DID lookups (copied from global on init) + authorizer auth.HoldAuthorizer // Hold authorization (copied from global on init) + webhookDispatcher storage.PushWebhookDispatcher // Push webhook dispatcher (copied from global on init) + manifestRefChecker storage.ManifestReferenceChecker // Manifest reference checker (copied from global on init) + validationCache *validationCache // Request-level service token cache + readmeFetcher *readme.Fetcher // README fetcher for repo pages + userPrefs UserPrefsCache // Cached sailor profile preferences (copied from global on init) } // initATProtoResolver initializes the name resolution middleware @@ -288,18 +287,17 @@ func initATProtoResolver(ctx context.Context, ns distribution.Namespace, _ drive // Copy shared services from globals into the instance // This avoids accessing globals during request handling return &NamespaceResolver{ - Namespace: ns, - defaultHoldDID: defaultHoldDID, - baseURL: baseURL, - fallbackUnreachable: atproto.TestModeBuild, - refresher: globalRefresher, - database: globalDatabase, - authorizer: globalAuthorizer, - webhookDispatcher: globalWebhookDispatcher, - manifestRefChecker: globalManifestRefChecker, - validationCache: newValidationCache(), - readmeFetcher: readme.NewFetcher(), - userPrefs: globalUserPrefs, + Namespace: ns, + defaultHoldDID: defaultHoldDID, + baseURL: baseURL, + refresher: globalRefresher, + database: globalDatabase, + authorizer: globalAuthorizer, + webhookDispatcher: globalWebhookDispatcher, + manifestRefChecker: globalManifestRefChecker, + validationCache: newValidationCache(), + readmeFetcher: readme.NewFetcher(), + userPrefs: globalUserPrefs, }, nil } @@ -708,7 +706,7 @@ func (nr *NamespaceResolver) findHoldDIDAndPrefs(ctx context.Context, did, handl // Both fields are known locally. The cached defaultHold was already // normalized to a DID by whoever wrote it, so no URL-to-DID // migration is needed on this path. - return nr.applyTestModeFallback(ctx, prefs.DefaultHoldDID), + return nr.holdOrDefault(prefs.DefaultHoldDID), holdPrefs{AutoRemoveUntagged: prefs.AutoRemoveUntagged.Bool} } } @@ -759,22 +757,15 @@ func (nr *NamespaceResolver) learnHoldPrefs(ctx context.Context, did, handle, pd } } - return nr.applyTestModeFallback(ctx, holdDID), holdPrefs{AutoRemoveUntagged: autoRemove} + return nr.holdOrDefault(holdDID), holdPrefs{AutoRemoveUntagged: autoRemove} } -// applyTestModeFallback turns a user's chosen hold into the hold to actually -// use. An empty choice means the appview default. With fallbackUnreachable set -// (testmode builds) a chosen hold that is not answering also falls back, so a -// developer whose local hold is down can still push. -func (nr *NamespaceResolver) applyTestModeFallback(ctx context.Context, userHoldDID string) string { +// holdOrDefault turns a user's chosen hold into the hold to actually use. An +// empty choice means the appview default. +func (nr *NamespaceResolver) holdOrDefault(userHoldDID string) string { if userHoldDID == "" { return nr.defaultHoldDID } - if nr.fallbackUnreachable && !nr.isHoldReachable(ctx, userHoldDID) { - slog.Debug("User's defaultHold unreachable, falling back to default", - "component", "registry/middleware/testmode", "default_hold", userHoldDID) - return nr.defaultHoldDID - } return userHoldDID } @@ -799,21 +790,6 @@ func (nr *NamespaceResolver) resolveSuccessor(ctx context.Context, holdDID strin return holdDID } -// isHoldReachable checks if a hold service is reachable -// Used in test mode to fallback to default hold when user's hold is unavailable -func (nr *NamespaceResolver) isHoldReachable(ctx context.Context, holdDID string) bool { - holdURL, err := atproto.ResolveHoldURL(ctx, holdDID) - if err != nil { - slog.Debug("Cannot resolve hold URL for reachability check", "component", "registry/middleware", "holdDID", holdDID, "error", err) - return false - } - - testURL := holdURL + "/.well-known/did.json" - client := atproto.NewClient("", "", "") - _, err = client.FetchDIDDocument(ctx, testURL) - return err == nil -} - // ExtractAuthMethod is an HTTP middleware that extracts the auth method and puller DID from the JWT Authorization header // and stores them in the request context for later use by the registry middleware. // Also stores the HTTP method for routing decisions (GET/HEAD = pull, PUT/POST = push). diff --git a/pkg/appview/middleware/registry_test.go b/pkg/appview/middleware/registry_test.go index 18ed7eb..e4f5376 100644 --- a/pkg/appview/middleware/registry_test.go +++ b/pkg/appview/middleware/registry_test.go @@ -109,7 +109,6 @@ func TestInitATProtoResolver(t *testing.T) { if baseURL, ok := tt.options["base_url"].(string); ok { assert.Equal(t, baseURL, resolver.baseURL) } - assert.Equal(t, atproto.TestModeBuild, resolver.fallbackUnreachable) }) } } @@ -211,66 +210,6 @@ func TestFindHoldDID_Priority(t *testing.T) { assert.Equal(t, "did:web:profile.hold.io", holdDID, "should prioritize sailor profile over hold records") } -// TestFindHoldDID_TestModeFallback tests the testmode-build fallback when the hold is unreachable -func TestFindHoldDID_TestModeFallback(t *testing.T) { - // Start a mock PDS server that returns a profile with unreachable hold - mockPDS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/xrpc/com.atproto.repo.getRecord" { - // Return sailor profile with an unreachable hold - profile := atproto.NewSailorProfileRecord("did:web:unreachable.hold.io") - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "value": profile, - }) - return - } - w.WriteHeader(http.StatusNotFound) - })) - defer mockPDS.Close() - - resolver := &NamespaceResolver{ - defaultHoldDID: "did:web:default.atcr.io", - fallbackUnreachable: true, // what a testmode build sets - } - - ctx := context.Background() - holdDID, _ := resolver.findHoldDIDAndPrefs(ctx, "did:plc:test123", "test.example.com", mockPDS.URL) - - // In a testmode build with an unreachable hold, should fall back to default - assert.Equal(t, "did:web:default.atcr.io", holdDID, "should fall back to default in a testmode build when hold unreachable") -} - -// TestIsHoldReachable tests the hold reachability check -func TestIsHoldReachable(t *testing.T) { - // Mock hold server with DID document - mockHold := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/.well-known/did.json" { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "id": "did:web:reachable.hold.io", - }) - return - } - w.WriteHeader(http.StatusNotFound) - })) - defer mockHold.Close() - - resolver := &NamespaceResolver{} - - ctx := context.Background() - - t.Run("reachable hold", func(t *testing.T) { - // Use URL format directly — DID resolution requires real identity directory - reachable := resolver.isHoldReachable(ctx, mockHold.URL) - assert.True(t, reachable, "should detect reachable hold") - }) - - t.Run("unreachable hold", func(t *testing.T) { - reachable := resolver.isHoldReachable(ctx, "did:web:nonexistent.example.com") - assert.False(t, reachable, "should detect unreachable hold") - }) -} - // TestRepositoryCaching tests that repositories are cached by DID+name func TestRepositoryCaching(t *testing.T) { // This test requires integration with actual repository resolution