clean up old migration code. minor bug fixes with appview ui

2026-05-13 03:21:31 +00:00 · 2026-05-04 21:52:28 -05:00
parent 1ac8af74d5
commit b2d6842bb7
21 changed files with 445 additions and 385 deletions
--- a/config-appview.example.yaml
+++ b/config-appview.example.yaml
@@ -29,8 +29,6 @@ server:
    default_hold_did: ""
    # Allows HTTP (not HTTPS) for DID resolution and uses transition:generic OAuth scope.
    test_mode: false
-    # Path to P-256 private key for OAuth client authentication. Auto-generated on first run.
-    oauth_key_path: /var/lib/atcr/oauth/client.key
    # Display name shown on OAuth authorization screens.
    client_name: AT Container Registry
    # Short name used in page titles and browser tabs.
@@ -45,7 +43,7 @@ ui:
    # SQLite/libSQL database for OAuth sessions, stars, pull counts, and device approvals.
    database_path: /var/lib/atcr/ui.db
    # Visual theme name (e.g. "seamark"). Empty uses default atcr.io branding.
-    theme: "seamark"
+    theme: ""
    # libSQL sync URL (libsql://...). Works with Turso cloud or self-hosted libsql-server. Leave empty for local-only SQLite.
    libsql_sync_url: ""
    # Auth token for libSQL sync. Required if libsql_sync_url is set.
--- a/deploy/upcloud/configs/appview.yaml.tmpl
+++ b/deploy/upcloud/configs/appview.yaml.tmpl
@@ -11,7 +11,6 @@ server:
    addr: :5000
    base_url: "https://seamark.dev"
    default_hold_did: "{{.HoldDid}}"
-    oauth_key_path: "{{.BasePath}}/oauth/client.key"
    client_name: Seamark
    test_mode: false
    client_short_name: Seamark
@@ -40,7 +39,6 @@ jetstream:
        - https://relay1.us-east.bsky.network
        - https://relay1.us-west.bsky.network
 auth:
-    key_path: "{{.BasePath}}/auth/private-key.pem"
    cert_path: "{{.BasePath}}/auth/private-key.crt"
 legal:
    company_name: Seamark
--- a/pkg/appview/config.go
+++ b/pkg/appview/config.go
@@ -52,9 +52,6 @@ type ServerConfig struct {
 	// Allows HTTP (not HTTPS) for DID resolution.
 	TestMode bool `yaml:"test_mode" comment:"Allows HTTP (not HTTPS) for DID resolution and uses transition:generic OAuth scope."`

-	// Path to P-256 private key for OAuth client authentication.
-	OAuthKeyPath string `yaml:"oauth_key_path" comment:"Path to P-256 private key for OAuth client authentication. Auto-generated on first run."`
-
 	// Display name shown on OAuth authorization screens.
 	ClientName string `yaml:"client_name" comment:"Display name shown on OAuth authorization screens."`

@@ -116,11 +113,8 @@ type JetstreamConfig struct {

 // AuthConfig defines authentication settings
 type AuthConfig struct {
-	// RSA private key for signing registry JWTs.
-	KeyPath string `yaml:"key_path" comment:"RSA private key for signing registry JWTs issued to Docker clients."`
-
 	// X.509 certificate matching the JWT signing key.
-	CertPath string `yaml:"cert_path" comment:"X.509 certificate matching the JWT signing key."`
+	CertPath string `yaml:"cert_path" comment:"X.509 certificate matching the JWT signing key (auto-generated on each boot from the JWT key in the database)."`

 	// TokenExpiration is the JWT expiration duration (5 minutes, not configurable)
 	TokenExpiration time.Duration `yaml:"-"`
@@ -169,7 +163,6 @@ func setDefaults(v *viper.Viper) {
 	v.SetDefault("server.test_mode", false)
 	v.SetDefault("server.client_name", "AT Container Registry")
 	v.SetDefault("server.client_short_name", "ATCR")
-	v.SetDefault("server.oauth_key_path", "/var/lib/atcr/oauth/client.key")
 	v.SetDefault("server.registry_domains", []string{})
 	v.SetDefault("server.managed_holds", []string{})

@@ -200,7 +193,6 @@ func setDefaults(v *viper.Viper) {
 	})

 	// Auth defaults
-	v.SetDefault("auth.key_path", "/var/lib/atcr/auth/private-key.pem")
 	v.SetDefault("auth.cert_path", "/var/lib/atcr/auth/private-key.crt")

 	// Log shipper defaults
--- a/pkg/appview/crypto_keys.go
+++ b/pkg/appview/crypto_keys.go
@@ -18,10 +18,8 @@ import (
 	"github.com/bluesky-social/indigo/atproto/atcrypto"
 )

-// loadOAuthKey loads the OAuth P-256 key with priority: DB → file → generate.
-// Keys loaded from file or newly generated are stored in the DB.
-func loadOAuthKey(database *sql.DB, keyPath string) (*atcrypto.PrivateKeyP256, error) {
-	// Try database first
+// loadOAuthKey loads the OAuth P-256 key from the DB, generating one if absent.
+func loadOAuthKey(database *sql.DB) (*atcrypto.PrivateKeyP256, error) {
 	data, err := db.GetCryptoKey(database, "oauth_p256")
 	if err != nil {
 		return nil, fmt.Errorf("failed to query crypto_keys: %w", err)
@@ -35,23 +33,6 @@ func loadOAuthKey(database *sql.DB, keyPath string) (*atcrypto.PrivateKeyP256, e
 		return key, nil
 	}

-	// Try file fallback
-	if keyPath != "" {
-		if fileData, err := os.ReadFile(keyPath); err == nil {
-			key, err := atcrypto.ParsePrivateBytesP256(fileData)
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse OAuth key from file %s: %w", keyPath, err)
-			}
-			// Migrate to database
-			if err := db.PutCryptoKey(database, "oauth_p256", fileData); err != nil {
-				return nil, fmt.Errorf("failed to store OAuth key in database: %w", err)
-			}
-			slog.Info("Migrated OAuth P-256 key from file to database", "path", keyPath)
-			return key, nil
-		}
-	}
-
-	// Generate new key
 	p256Key, err := atcrypto.GeneratePrivateKeyP256()
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate OAuth P-256 key: %w", err)
@@ -66,16 +47,15 @@ func loadOAuthKey(database *sql.DB, keyPath string) (*atcrypto.PrivateKeyP256, e
 	return p256Key, nil
 }

-// loadJWTKeyAndCert loads the JWT RSA key from DB (with file fallback) and generates
-// a self-signed certificate. The cert is always regenerated and written to certPath
-// on disk because the distribution library reads it via os.Open().
-func loadJWTKeyAndCert(database *sql.DB, keyPath, certPath string) (*rsa.PrivateKey, []byte, error) {
-	rsaKey, err := loadRSAKey(database, keyPath)
+// loadJWTKeyAndCert loads the JWT RSA key from the DB and generates a self-signed
+// certificate. The cert is always regenerated and written to certPath on disk
+// because the distribution library reads it via os.Open().
+func loadJWTKeyAndCert(database *sql.DB, certPath string) (*rsa.PrivateKey, []byte, error) {
+	rsaKey, err := loadRSAKey(database)
 	if err != nil {
 		return nil, nil, err
 	}

-	// Generate cert and write to disk for distribution library
 	certDER, err := generateAndWriteCert(rsaKey, certPath)
 	if err != nil {
 		return nil, nil, err
@@ -84,9 +64,8 @@ func loadJWTKeyAndCert(database *sql.DB, keyPath, certPath string) (*rsa.Private
 	return rsaKey, certDER, nil
 }

-// loadRSAKey loads the RSA private key with priority: DB → file → generate.
-func loadRSAKey(database *sql.DB, keyPath string) (*rsa.PrivateKey, error) {
-	// Try database first
+// loadRSAKey loads the RSA private key from the DB, generating one if absent.
+func loadRSAKey(database *sql.DB) (*rsa.PrivateKey, error) {
 	data, err := db.GetCryptoKey(database, "jwt_rsa")
 	if err != nil {
 		return nil, fmt.Errorf("failed to query crypto_keys: %w", err)
@@ -100,23 +79,6 @@ func loadRSAKey(database *sql.DB, keyPath string) (*rsa.PrivateKey, error) {
 		return key, nil
 	}

-	// Try file fallback
-	if keyPath != "" {
-		if fileData, err := os.ReadFile(keyPath); err == nil {
-			key, err := parseRSAKeyPEM(fileData)
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse RSA key from file %s: %w", keyPath, err)
-			}
-			// Migrate to database
-			if err := db.PutCryptoKey(database, "jwt_rsa", fileData); err != nil {
-				return nil, fmt.Errorf("failed to store RSA key in database: %w", err)
-			}
-			slog.Info("Migrated JWT RSA key from file to database", "path", keyPath)
-			return key, nil
-		}
-	}
-
-	// Generate new key
 	rsaKey, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate RSA key: %w", err)
--- a/pkg/appview/jetstream/processor_test.go
+++ b/pkg/appview/jetstream/processor_test.go
@@ -489,43 +489,6 @@ func TestProcessStar(t *testing.T) {
 	}
 }

-func TestProcessStar_OldFormat(t *testing.T) {
-	database := setupTestDB(t)
-	defer database.Close()
-
-	// Insert test users
-	for _, did := range []string{"did:plc:starrer456", "did:plc:owner456"} {
-		_, err := database.Exec(
-			`INSERT INTO users (did, handle, pds_endpoint, last_seen) VALUES (?, ?, ?, ?)`,
-			did, did+".test", "https://pds.example.com", time.Now())
-		if err != nil {
-			t.Fatalf("Failed to insert test user %s: %v", did, err)
-		}
-	}
-
-	p := NewProcessor(database, false, nil)
-	ctx := context.Background()
-
-	// Old format JSON (object subject with did + repository)
-	oldFormatJSON := `{"$type":"io.atcr.sailor.star","subject":{"did":"did:plc:owner456","repository":"legacy-app"},"createdAt":"2025-06-01T00:00:00Z"}`
-
-	err := p.ProcessStar(ctx, "did:plc:starrer456", []byte(oldFormatJSON))
-	if err != nil {
-		t.Fatalf("ProcessStar (old format) failed: %v", err)
-	}
-
-	// Verify star was inserted with correct owner/repo
-	var count int
-	err = database.QueryRow("SELECT COUNT(*) FROM stars WHERE starrer_did = ? AND owner_did = ? AND repository = ?",
-		"did:plc:starrer456", "did:plc:owner456", "legacy-app").Scan(&count)
-	if err != nil {
-		t.Fatalf("Failed to query stars: %v", err)
-	}
-	if count != 1 {
-		t.Errorf("Expected 1 star from old format, got %d", count)
-	}
-}
-
 func TestProcessManifest_Duplicate(t *testing.T) {
 	database := setupTestDB(t)
 	defer database.Close()
--- a/pkg/appview/public/js/bundle.min.js
+++ b/pkg/appview/public/js/bundle.min.js
--- a/pkg/appview/server.go
+++ b/pkg/appview/server.go
@@ -202,8 +202,7 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,
 		atproto.SetTestMode(true)
 	}

-	// Load crypto keys from database (with file fallback and migration)
-	oauthKey, err := loadOAuthKey(s.Database, cfg.Server.OAuthKeyPath)
+	oauthKey, err := loadOAuthKey(s.Database)
 	if err != nil {
 		return nil, fmt.Errorf("failed to load OAuth key: %w", err)
 	}
@@ -455,17 +454,6 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,
 			}(client, s.Refresher, holdDID, s.HoldAuthorizer)
 		}

-		// Migrate old-format star records to AT URI format in background
-		go func(client *atproto.Client, did string) {
-			ctx := context.Background()
-			migrated, err := atproto.MigrateStarRecords(ctx, client)
-			if err != nil {
-				slog.Warn("Star record migration failed", "component", "appview/callback", "did", did, "error", err, "migrated", migrated)
-			} else if migrated > 0 {
-				slog.Info("Migrated star records to AT URI format", "component", "appview/callback", "did", did, "count", migrated)
-			}
-		}(client, did)
-
 		// Drain manifests from old hold to successor in background
 		go func(client *atproto.Client, did string) {
 			ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
@@ -485,7 +473,7 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,

 	// Create token issuer
 	if cfg.Distribution.Auth["token"] != nil {
-		rsaKey, certDER, err := loadJWTKeyAndCert(s.Database, cfg.Auth.KeyPath, cfg.Auth.CertPath)
+		rsaKey, certDER, err := loadJWTKeyAndCert(s.Database, cfg.Auth.CertPath)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load JWT key material: %w", err)
 		}
--- a/pkg/appview/src/js/app.js
+++ b/pkg/appview/src/js/app.js
@@ -138,7 +138,7 @@ document.addEventListener('DOMContentLoaded', () => {
            if (tag === 'INPUT' || tag === 'TEXTAREA' || e.target.isContentEditable) return;

            e.preventDefault();
-            wrapper.classList.add('expanded');
+            setSearchExpanded(wrapper, true);
            input.focus();
        }
    });
--- a/pkg/appview/storage/crew.go
+++ b/pkg/appview/storage/crew.go
@@ -29,6 +29,18 @@ func EnsureCrewMembership(ctx context.Context, client *atproto.Client, refresher
 		return
 	}

+	// Short-circuit if we have a recent cached approval. The hold's requestCrew
+	// would just return success for an established crew member, so the round-trip
+	// is pure waste. Cache lookup uses the same key (holdDID, userDID) that
+	// CheckWriteAccess will hit later in the request.
+	if authorizer != nil {
+		if cached, err := authorizer.IsCachedCrewMember(ctx, holdDID, client.DID()); err == nil && cached {
+			slog.Debug("crew membership cached, skipping requestCrew",
+				"holdDID", holdDID, "userDID", client.DID())
+			return
+		}
+	}
+
 	// Resolve hold DID to HTTP endpoint
 	holdEndpoint, err := atproto.ResolveHoldURL(ctx, holdDID)
 	if err != nil {
@@ -61,8 +73,17 @@ func EnsureCrewMembership(ctx context.Context, client *atproto.Client, refresher

 	slog.Info("successfully registered as crew member", "holdDID", holdDID, "userDID", client.DID())

-	// Clear any cached denial to ensure immediate access
 	if authorizer != nil {
+		// Warm the approval cache so subsequent CheckWriteAccess calls within this
+		// request (e.g. each layer in a multi-layer push) skip the XRPC getRecord.
+		if err := authorizer.RecordCrewApproval(ctx, holdDID, client.DID()); err != nil {
+			slog.Warn("failed to record crew approval after crew registration",
+				"holdDID", holdDID,
+				"userDID", client.DID(),
+				"error", err)
+		}
+
+		// Clear any cached denial to ensure immediate access
 		if err := authorizer.ClearCrewDenial(ctx, holdDID, client.DID()); err != nil {
 			slog.Warn("failed to clear denial cache after crew registration",
 				"holdDID", holdDID,
--- a/pkg/appview/storage/crew_test.go
+++ b/pkg/appview/storage/crew_test.go
@@ -2,7 +2,11 @@ package storage

 import (
 	"context"
+	"sync/atomic"
 	"testing"
+
+	"atcr.io/pkg/atproto"
+	"atcr.io/pkg/auth"
 )

 func TestEnsureCrewMembership_EmptyHoldDID(t *testing.T) {
@@ -11,4 +15,83 @@ func TestEnsureCrewMembership_EmptyHoldDID(t *testing.T) {
 	// If we get here without panic, test passes
 }

-// TODO: Add comprehensive tests with HTTP client mocking
+// fakeAuthorizer records cache calls for use in EnsureCrewMembership tests.
+type fakeAuthorizer struct {
+	cachedReturn        bool
+	isCachedCalls       atomic.Int32
+	recordApprovalCalls atomic.Int32
+	clearDenialCalls    atomic.Int32
+}
+
+func (f *fakeAuthorizer) GetCaptainRecord(ctx context.Context, holdDID string) (*atproto.CaptainRecord, error) {
+	return nil, nil
+}
+func (f *fakeAuthorizer) CheckReadAccess(ctx context.Context, holdDID, userDID string) (bool, error) {
+	return false, nil
+}
+func (f *fakeAuthorizer) CheckWriteAccess(ctx context.Context, holdDID, userDID string) (bool, error) {
+	return false, nil
+}
+func (f *fakeAuthorizer) IsCrewMember(ctx context.Context, holdDID, userDID string) (bool, error) {
+	return false, nil
+}
+func (f *fakeAuthorizer) ClearCrewDenial(ctx context.Context, holdDID, userDID string) error {
+	f.clearDenialCalls.Add(1)
+	return nil
+}
+func (f *fakeAuthorizer) IsCachedCrewMember(ctx context.Context, holdDID, userDID string) (bool, error) {
+	f.isCachedCalls.Add(1)
+	return f.cachedReturn, nil
+}
+func (f *fakeAuthorizer) RecordCrewApproval(ctx context.Context, holdDID, userDID string) error {
+	f.recordApprovalCalls.Add(1)
+	return nil
+}
+
+var _ auth.HoldAuthorizer = (*fakeAuthorizer)(nil)
+
+// TestEnsureCrewMembership_SkipsRequestCrewWhenCached verifies the cache short-circuit:
+// when IsCachedCrewMember returns true, the function returns before attempting the
+// requestCrew POST (and before fetching a service token, which would otherwise fail
+// with a nil refresher).
+func TestEnsureCrewMembership_SkipsRequestCrewWhenCached(t *testing.T) {
+	authz := &fakeAuthorizer{cachedReturn: true}
+	client := atproto.NewClient("https://pds.example", "did:plc:user123", "")
+	holdDID := "did:web:hold01.atcr.io"
+
+	// Pass nil refresher: if the cache check did NOT short-circuit, the function
+	// would log "skipping crew registration" and still return without panic — but
+	// it also would NOT have called IsCachedCrewMember, which is what we assert.
+	EnsureCrewMembership(context.Background(), client, nil, holdDID, authz)
+
+	if authz.isCachedCalls.Load() != 1 {
+		t.Errorf("Expected IsCachedCrewMember to be called once, got %d", authz.isCachedCalls.Load())
+	}
+	if authz.recordApprovalCalls.Load() != 0 {
+		t.Errorf("Expected RecordCrewApproval not to be called on cache hit, got %d", authz.recordApprovalCalls.Load())
+	}
+	if authz.clearDenialCalls.Load() != 0 {
+		t.Errorf("Expected ClearCrewDenial not to be called on cache hit, got %d", authz.clearDenialCalls.Load())
+	}
+}
+
+// TestEnsureCrewMembership_NoRefresherFallsThroughCacheCheck verifies that a cache
+// miss with a nil refresher takes the existing app-password skip path, and does not
+// call RecordCrewApproval (since requestCrew never ran).
+func TestEnsureCrewMembership_NoRefresherFallsThroughCacheCheck(t *testing.T) {
+	authz := &fakeAuthorizer{cachedReturn: false}
+	client := atproto.NewClient("https://pds.example", "did:plc:user123", "")
+	holdDID := "did:web:hold01.atcr.io"
+
+	EnsureCrewMembership(context.Background(), client, nil, holdDID, authz)
+
+	if authz.isCachedCalls.Load() != 1 {
+		t.Errorf("Expected IsCachedCrewMember to be called once, got %d", authz.isCachedCalls.Load())
+	}
+	if authz.recordApprovalCalls.Load() != 0 {
+		t.Errorf("Expected RecordCrewApproval not to be called when requestCrew did not run, got %d", authz.recordApprovalCalls.Load())
+	}
+}
+
+// TODO: Add comprehensive tests with HTTP client mocking for the requestCrew success
+// path (requires a working oauth.Refresher to obtain a service token).
--- a/pkg/appview/storage/profile.go
+++ b/pkg/appview/storage/profile.go
@@ -19,40 +19,62 @@ const ProfileRKey = "self"
 // Used to prevent duplicate migration goroutines
 var migrationLocks sync.Map

-// EnsureProfile checks if a user's profile exists and creates it if needed
-// This should be called during authentication (OAuth exchange or token service)
-// If defaultHoldDID is provided, creates profile with that default (or empty if not provided)
-// Expected format: "did:web:hold01.atcr.io"
-// Normalizes URLs to DIDs for consistency (for backward compatibility)
+// EnsureProfile checks if a user's profile exists and creates it if needed.
+// If the profile already exists, missing fields are reconciled against the
+// AppView's defaults so downstream logic (e.g. successor migration) has a
+// concrete defaultHold to anchor on instead of relying on the empty fallback.
+// Currently only defaultHold is reconciled; other zero-valued fields have
+// their own runtime defaults and shouldn't be written unprompted.
+// This should be called during authentication (OAuth exchange or token service).
+// Expected format for defaultHoldDID: "did:web:hold01.atcr.io"
 func EnsureProfile(ctx context.Context, client *atproto.Client, defaultHoldDID string) error {
-	// Check if profile already exists
-	profile, err := client.GetRecord(ctx, atproto.SailorProfileCollection, ProfileRKey)
-	if err == nil && profile != nil {
-		// Profile exists, nothing to do
-		return nil
-	}
-
-	// Normalize to DID if it's a URL (or pass through if already a DID)
-	// This ensures we store DIDs consistently in new profiles
+	// Resolve the AppView default to a DID up-front; used both for create and reconcile paths.
 	normalizedDID := ""
 	if defaultHoldDID != "" {
 		resolved, err := atproto.ResolveHoldDID(ctx, defaultHoldDID)
 		if err != nil {
-			slog.Warn("Failed to resolve hold DID for new profile", "component", "profile", "defaultHold", defaultHoldDID, "error", err)
+			slog.Warn("Failed to resolve hold DID", "component", "profile", "defaultHold", defaultHoldDID, "error", err)
 		} else {
 			normalizedDID = resolved
 		}
 	}

-	// Profile doesn't exist - create it
-	newProfile := atproto.NewSailorProfileRecord(normalizedDID)
-
-	_, err = client.PutRecord(ctx, atproto.SailorProfileCollection, ProfileRKey, newProfile)
-	if err != nil {
-		return fmt.Errorf("failed to create sailor profile: %w", err)
+	record, err := client.GetRecord(ctx, atproto.SailorProfileCollection, ProfileRKey)
+	if err != nil && !errors.Is(err, atproto.ErrRecordNotFound) {
+		// Preserve previous best-effort behavior: log and treat as missing.
+		slog.Warn("Failed to fetch existing profile", "component", "profile", "did", client.DID(), "error", err)
+		record = nil
 	}

-	slog.Debug("Created sailor profile", "component", "profile", "default_hold", normalizedDID)
+	if record == nil {
+		newProfile := atproto.NewSailorProfileRecord(normalizedDID)
+		if _, err := client.PutRecord(ctx, atproto.SailorProfileCollection, ProfileRKey, newProfile); err != nil {
+			return fmt.Errorf("failed to create sailor profile: %w", err)
+		}
+		slog.Debug("Created sailor profile", "component", "profile", "default_hold", normalizedDID)
+		return nil
+	}
+
+	var profile atproto.SailorProfileRecord
+	if err := json.Unmarshal(record.Value, &profile); err != nil {
+		return fmt.Errorf("failed to parse existing profile: %w", err)
+	}
+
+	changed := false
+	if profile.DefaultHold == "" && normalizedDID != "" {
+		profile.DefaultHold = normalizedDID
+		changed = true
+		slog.Info("Reconciling empty defaultHold to AppView default", "component", "profile", "did", client.DID(), "default_hold", normalizedDID)
+	}
+
+	if !changed {
+		return nil
+	}
+
+	profile.UpdatedAt = time.Now()
+	if _, err := client.PutRecord(ctx, atproto.SailorProfileCollection, ProfileRKey, &profile); err != nil {
+		return fmt.Errorf("failed to reconcile sailor profile: %w", err)
+	}
 	return nil
 }

--- a/pkg/appview/storage/profile_test.go
+++ b/pkg/appview/storage/profile_test.go
@@ -203,6 +203,99 @@ func TestEnsureProfile_Exists(t *testing.T) {
 	}
 }

+// TestEnsureProfile_ReconcilesEmptyDefaultHold tests that an existing profile
+// with empty defaultHold gets backfilled with the AppView default. This anchors
+// users (typically app-password-only ones who never log in interactively) to a
+// concrete hold so successor migration / drain has a candidate to work with.
+func TestEnsureProfile_ReconcilesEmptyDefaultHold(t *testing.T) {
+	var sentProfile map[string]any
+	var mu sync.Mutex
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// GetRecord: profile exists, defaultHold is empty
+		if r.Method == "GET" {
+			response := `{
+				"uri": "at://did:plc:test123/io.atcr.sailor.profile/self",
+				"cid": "bafytest",
+				"value": {
+					"$type": "io.atcr.sailor.profile",
+					"createdAt": "2025-01-01T00:00:00Z",
+					"updatedAt": "2025-01-01T00:00:00Z"
+				}
+			}`
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(response))
+			return
+		}
+
+		// PutRecord: reconciliation should write
+		if r.Method == "POST" && strings.Contains(r.URL.Path, "putRecord") {
+			var body map[string]any
+			json.NewDecoder(r.Body).Decode(&body)
+			mu.Lock()
+			sentProfile = body
+			mu.Unlock()
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.sailor.profile/self","cid":"bafytest"}`))
+			return
+		}
+	}))
+	defer server.Close()
+
+	client := atproto.NewClient(server.URL, "did:plc:test123", "test-token")
+	if err := EnsureProfile(context.Background(), client, "did:web:hold01.atcr.io"); err != nil {
+		t.Fatalf("EnsureProfile() error = %v", err)
+	}
+
+	mu.Lock()
+	defer mu.Unlock()
+	if sentProfile == nil {
+		t.Fatal("expected reconciliation PutRecord call")
+	}
+	recordData := sentProfile["record"].(map[string]any)
+	if got := recordData["defaultHold"]; got != "did:web:hold01.atcr.io" {
+		t.Errorf("defaultHold after reconcile = %v, want did:web:hold01.atcr.io", got)
+	}
+}
+
+// TestEnsureProfile_NoReconcileWhenSet asserts we don't churn existing profiles
+// that already have a defaultHold (regardless of whether it matches the AppView
+// default — that's the user's explicit choice).
+func TestEnsureProfile_NoReconcileWhenSet(t *testing.T) {
+	putRecordCalled := false
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == "GET" {
+			response := `{
+				"uri": "at://did:plc:test123/io.atcr.sailor.profile/self",
+				"cid": "bafytest",
+				"value": {
+					"$type": "io.atcr.sailor.profile",
+					"defaultHold": "did:plc:userpicked",
+					"createdAt": "2025-01-01T00:00:00Z",
+					"updatedAt": "2025-01-01T00:00:00Z"
+				}
+			}`
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte(response))
+			return
+		}
+		if r.Method == "POST" && strings.Contains(r.URL.Path, "putRecord") {
+			putRecordCalled = true
+			t.Error("PutRecord should not be called when defaultHold is already set")
+		}
+	}))
+	defer server.Close()
+
+	client := atproto.NewClient(server.URL, "did:plc:test123", "test-token")
+	if err := EnsureProfile(context.Background(), client, "did:web:hold01.atcr.io"); err != nil {
+		t.Fatalf("EnsureProfile() error = %v", err)
+	}
+	if putRecordCalled {
+		t.Error("unexpected PutRecord call")
+	}
+}
+
 // TestGetProfile tests retrieving a user's profile
 func TestGetProfile(t *testing.T) {
 	tests := []struct {
--- a/pkg/appview/templates/components/nav-brand.html
+++ b/pkg/appview/templates/components/nav-brand.html
@@ -1,11 +1,6 @@
 {{ define "nav-brand" }}
 <a href="/" class="flex items-center gap-2 text-2xl font-display font-bold text-base-content no-underline tracking-tight focus-visible:outline-2 focus-visible:outline-primary focus-visible:outline-offset-2 rounded-sm">
-    <svg class="nav-brand-logo h-12 w-auto" viewBox="-77.86658 0 736.12415 736.12415" width="48" height="48" aria-hidden="true" focusable="false">
-        <g transform="translate(-221.72255,-136.52091)">
-            <path fill="#fdfdfd" d="M 617.5495,871.8604 C 578.11386,869.03851 541.82227,858.4385 500.12778,837.56397 465.36235,820.15851 436.76514,811.07059 403.5,806.85656 c -12.84516,-1.62722 -49.07301,-1.60244 -62,0.0424 -33.91367,4.31524 -65.77468,15.2698 -95.98413,33.00161 -9.12922,5.35851 -13.94133,5.5727 -19.08341,0.84942 -5.98616,-5.4986 -6.2808,-13.15645 -0.72998,-18.97261 3.25811,-3.41385 18.13488,-11.95914 32.64892,-18.7537 20.1575,-9.43648 48.25209,-17.77664 72.6486,-21.56648 16.14087,-2.50738 56.00489,-3.09183 73.35439,-1.07546 38.90127,4.52113 66.90782,13.35774 110.65324,34.91324 32.03372,15.78457 54.63895,23.156 85.328,27.82494 17.10618,2.60249 48.21475,3.55343 64.17738,1.9618 39.21587,-3.91019 79.89818,-17.74495 111.98699,-38.0832 9.49605,-6.01871 11.46031,-6.6266 16.5169,-5.11161 7.93123,2.37625 11.12157,9.92854 7.78731,18.43441 -2.64662,6.75168 -37.55875,25.98896 -64.92934,35.77736 -37.43027,13.386 -77.3022,18.6972 -118.32537,15.76171 z M 618,805.41223 c -1.925,-0.22171 -7.775,-0.8899 -13,-1.48485 -21.24321,-2.41892 -50.05582,-9.3996 -70.2951,-17.03102 C 530.1922,785.1948 515.025,778.25587 501,771.47651 471.8442,757.38326 456.97198,751.58624 437.3313,746.65917 411.89585,740.27843 404.77,739.49571 372.5,739.538 c -22.47927,0.0295 -31.33872,0.42211 -39.40113,1.74626 -29.96332,4.92111 -60.43175,15.82046 -84.68974,30.29572 -10.43562,6.22716 -14.40931,7.02265 -20.17816,4.03946 -5.94688,-3.07525 -8.46008,-12.56844 -4.80438,-18.14775 3.66647,-5.59574 29.31775,-19.52383 51.07341,-27.73175 24.58467,-9.27525 43.08892,-13.51256 71,-16.25837 31.52697,-3.10152 63.72313,-0.73488 96.09111,7.06336 13.26193,3.19513 29.91602,8.61443 39.90889,12.98649 1.65,0.72191 5.25,2.20206 8,3.28923 2.75,1.08718 14.45,6.64838 26,12.35823 11.55,5.70985 23.25,11.12029 26,12.02321 2.75,0.90292 8.01923,2.7551 11.70941,4.11596 30.31402,11.17918 74.29895,16.77162 107.36691,13.65112 43.37883,-4.09349 80.37395,-16.50011 117.3777,-39.36357 8.46646,-5.23116 13.48207,-5.95296 18.47855,-2.65926 3.98502,2.62694 5.48994,5.68112 5.53199,11.22693 0.0474,6.25411 -2.48022,9.11099 -14.79473,16.72177 -30.30214,18.72775 -68.52877,32.58103 -105.50546,38.23503 -12.567,1.92158 -53.92342,3.40408 -63.66437,2.28216 z"/>
-            <path fill="#f9d911" d="m 617.5,778.92769 c -22.78112,-2.29598 -48.20714,-7.6784 -64.29059,-13.60964 -3.69018,-1.36086 -8.95941,-3.21304 -11.70941,-4.11596 -2.75,-0.90292 -14.45,-6.31336 -26,-12.02321 -11.55,-5.70985 -23.25,-11.27105 -26,-12.35823 -2.75,-1.08717 -6.35,-2.56732 -8,-3.28923 -9.99287,-4.37206 -26.64696,-9.79136 -39.90889,-12.98649 -32.36798,-7.79824 -64.56414,-10.16488 -96.09111,-7.06336 -14.17395,1.39439 -19.56653,2.1545 -30.75,4.33436 l -4.75,0.92586 0.008,-21.6209 c 0.004,-11.89149 0.4721,-24.48375 1.03986,-27.98279 1.90217,-11.72272 9.10364,-22.18111 18.9719,-27.55209 2.80287,-1.52551 14.63265,-6.51166 26.2884,-11.08032 17.63653,-6.91292 21.32151,-8.70088 21.96242,-10.65618 0.42358,-1.29223 3.70527,-19.67451 7.29266,-40.84951 10.93432,-64.54115 36.65053,-214.58556 47.9037,-279.5 1.90689,-11 4.40006,-25.625 5.54037,-32.5 1.1403,-6.875 2.31057,-13.2875 2.6006,-14.25 0.47966,-1.59334 -0.4683,-1.75 -10.59018,-1.75 C 416.06978,251 412,248.85761 412,240.98882 c 0,-2.14014 0.93248,-4.07514 2.92308,-6.06574 L 417.84615,232 h 94.62238 94.62238 l 2.45454,2.45455 c 3.63107,3.63106 3.42121,9.73263 -0.46853,13.62237 C 606.18281,250.97104 606.03914,251 594.57692,251 583.42789,251 583,251.0788 583,253.13192 c 0,1.17255 1.38641,10.06005 3.0809,19.75 4.02848,23.03676 22.95628,133.28638 39.93618,232.61808 3.76071,22 8.66683,50.58443 10.90248,63.52095 2.23566,12.93652 5.15946,29.98903 6.49734,37.89445 1.36905,8.08956 2.98675,14.71603 3.70023,15.15699 0.69723,0.43091 11.19361,4.71484 23.32528,9.51983 12.13168,4.80499 23.63509,9.72363 25.56316,10.93031 8.44769,5.28698 15.21532,16.07918 17.02955,27.1567 0.52431,3.20142 0.9559,26.82634 0.95909,52.49981 l 0.006,46.67904 -5.26963,1.53193 c -26.98949,7.84605 -63.85821,11.29637 -91.23037,8.53768 z M 616.00462,561.75 c 0.0111,-3.02045 -11.42676,-68.33901 -12.1049,-69.12748 -0.41508,-0.48262 -8.32482,1.69447 -17.5772,4.83798 -9.25239,3.1435 -52.73341,17.81377 -96.6245,32.60059 -43.89109,14.78682 -80.22859,27.14879 -80.75,27.47104 C 408.42661,557.85438 408,559.21648 408,560.55902 V 563 h 104 c 82.53087,0 104.00095,-0.25804 104.00462,-1.25 z M 503.64748,483.89936 c 47.21888,-16.00535 87.3399,-29.53337 89.1578,-30.06227 1.8179,-0.52889 3.55299,-1.60715 3.85575,-2.39613 0.4452,-1.16018 -3.19945,-25.14729 -5.18715,-34.13899 -0.46799,-2.11701 -2.89118,-1.38503 -65.72989,19.85499 -35.88419,12.12917 -73.34399,24.763 -83.24399,28.07518 -9.9,3.31219 -18.31011,6.30244 -18.68913,6.64501 -0.37902,0.34257 -2.07159,8.72285 -3.76128,18.62285 -1.68968,9.9 -3.30065,19.0125 -3.57992,20.25 -0.27928,1.2375 -0.0953,2.25 0.40876,2.25 0.50409,0 39.55016,-13.09529 86.76905,-29.10064 z m -12.00479,-76.80553 c 32.37152,-10.9484 66.24488,-22.4234 75.27412,-25.5 9.02925,-3.07661 16.56675,-5.59383 16.75,-5.59383 0.78989,0 0.2413,-6.30318 -0.70199,-8.06574 C 581.95652,366.05023 580.11419,366 512.02383,366 c -44.20346,0 -70.12676,0.35753 -70.50679,0.97242 -0.50686,0.82012 -10.47609,57.06549 -10.50667,59.27758 -0.006,0.4125 0.39134,0.75 0.88232,0.75 0.49098,0 27.37848,-8.95778 59.75,-19.90617 z M 464.69411,214.25 c 1.35047,-3.74335 4.751,-7.53803 7.95455,-8.87656 L 476,203.97316 v -17.27648 c 0,-19.53223 1.2134,-26.23741 6.13635,-33.90916 3.94773,-6.15201 8.94216,-10.46656 15.81802,-13.66476 4.57517,-2.12807 7.03676,-2.58325 14.06666,-2.60111 7.61634,-0.0194 9.22192,0.3234 15.12258,3.22828 7.83094,3.85516 13.97677,10.13512 17.68889,18.07492 2.52613,5.4031 2.67763,6.64616 3.16041,25.93123 l 0.50709,20.25609 3.48007,1.74391 C 555.13579,207.33747 560,213.03373 560,215.14788 560,215.61655 538.41412,216 512.03138,216 c -45.25823,0 -47.93294,-0.0989 -47.33727,-1.75 z"/>
-        </g>
-    </svg>
+    <img src="/favicon.svg" alt="" class="nav-brand-logo h-12 w-auto" aria-hidden="true" />
    <span class="min-w-0 max-w-56 truncate sm:max-w-none">{{ .ClientName }}</span>
 </a>
 {{ end }}
--- a/pkg/atproto/lexicon.go
+++ b/pkg/atproto/lexicon.go
@@ -3,7 +3,6 @@ package atproto
 //go:generate go run generate.go

 import (
-	"context"
 	"crypto/sha256"
 	"encoding/base32"
 	"encoding/base64"
@@ -434,45 +433,8 @@ type StarRecord struct {
 	CreatedAt time.Time `json:"createdAt"`
 }

-// UnmarshalJSON handles both old format (object subject) and new format (AT URI string subject)
-func (s *StarRecord) UnmarshalJSON(data []byte) error {
-	// Use a raw type to inspect the subject field
-	type starRecordRaw struct {
-		Type      string          `json:"$type"`
-		Subject   json.RawMessage `json:"subject"`
-		CreatedAt time.Time       `json:"createdAt"`
-	}
-	var raw starRecordRaw
-	if err := json.Unmarshal(data, &raw); err != nil {
-		return fmt.Errorf("failed to unmarshal star record: %w", err)
-	}
-
-	s.Type = raw.Type
-	s.CreatedAt = raw.CreatedAt
-
-	// Try new format: subject is a string AT URI
-	var subjectStr string
-	if err := json.Unmarshal(raw.Subject, &subjectStr); err == nil && strings.HasPrefix(subjectStr, "at://") {
-		s.Subject = subjectStr
-		return nil
-	}
-
-	// Fall back to old format: subject is an object with did + repository
-	var oldSubject struct {
-		DID        string `json:"did"`
-		Repository string `json:"repository"`
-	}
-	if err := json.Unmarshal(raw.Subject, &oldSubject); err != nil {
-		return fmt.Errorf("failed to unmarshal star subject (neither AT URI string nor {did, repository} object): %w", err)
-	}
-
-	s.Subject = BuildRepoPageURI(oldSubject.DID, oldSubject.Repository)
-	return nil
-}
-
 // GetSubjectDIDAndRepository extracts the owner DID and repository name
-// from the star record's subject AT URI. UnmarshalJSON normalizes old format
-// to AT URI, so this always works with ParseRepoPageURI.
+// from the star record's subject AT URI.
 func (s *StarRecord) GetSubjectDIDAndRepository() (ownerDID, repository string, err error) {
 	return ParseRepoPageURI(s.Subject)
 }
@@ -532,62 +494,6 @@ func ParseStarRecordKey(rkey string) (ownerDID, repository string, err error) {
 	return parts[0], parts[1], nil
 }

-// MigrateStarRecords lists the user's star records and rewrites any old-format
-// records (object subject with did/repository) to the new AT URI format.
-// Returns the number of records migrated.
-func MigrateStarRecords(ctx context.Context, client *Client) (int, error) {
-	migrated := 0
-	cursor := ""
-
-	for {
-		records, nextCursor, err := client.ListRecordsWithCursor(ctx, StarCollection, 100, cursor)
-		if err != nil {
-			return migrated, fmt.Errorf("failed to list star records: %w", err)
-		}
-
-		for _, rec := range records {
-			// Try to unmarshal as new format (string subject) — skip if already migrated
-			var subjectStr string
-			if err := json.Unmarshal(rec.Value, &struct {
-				Subject *string `json:"subject"`
-			}{Subject: &subjectStr}); err == nil && strings.HasPrefix(subjectStr, "at://") {
-				continue
-			}
-
-			// Old format — unmarshal via StarRecord (which normalizes to AT URI)
-			var starRecord StarRecord
-			if err := json.Unmarshal(rec.Value, &starRecord); err != nil {
-				continue
-			}
-
-			// Extract rkey from the record URI (at://did/collection/rkey)
-			uriParts := strings.Split(rec.URI, "/")
-			if len(uriParts) < 2 {
-				continue
-			}
-			rkey := uriParts[len(uriParts)-1]
-
-			// Rewrite with new format at the same rkey
-			newRecord := &StarRecord{
-				Type:      StarCollection,
-				Subject:   starRecord.Subject, // Already normalized to AT URI by UnmarshalJSON
-				CreatedAt: starRecord.CreatedAt,
-			}
-			if _, err := client.PutRecord(ctx, StarCollection, rkey, newRecord); err != nil {
-				return migrated, fmt.Errorf("failed to migrate star record %s: %w", rec.URI, err)
-			}
-			migrated++
-		}
-
-		if nextCursor == "" {
-			break
-		}
-		cursor = nextCursor
-	}
-
-	return migrated, nil
-}
-
 // IsDID checks if a string is a DID (starts with "did:")
 func IsDID(s string) bool {
 	return len(s) > 4 && s[:4] == "did:"
--- a/pkg/atproto/lexicon_test.go
+++ b/pkg/atproto/lexicon_test.go
@@ -812,52 +812,27 @@ func TestBlobReference_JSONSerialization(t *testing.T) {
 	}
 }

-func TestStarRecord_DualFormatDeserialization(t *testing.T) {
-	tests := []struct {
-		name         string
-		jsonData     string
-		wantSubject  string
-		wantOwnerDID string
-		wantRepo     string
-	}{
-		{
-			name:         "new format - AT URI string",
-			jsonData:     `{"$type":"io.atcr.sailor.star","subject":"at://did:plc:alice123/io.atcr.repo.page/myapp","createdAt":"2025-01-01T00:00:00Z"}`,
-			wantSubject:  "at://did:plc:alice123/io.atcr.repo.page/myapp",
-			wantOwnerDID: "did:plc:alice123",
-			wantRepo:     "myapp",
-		},
-		{
-			name:         "old format - object subject",
-			jsonData:     `{"$type":"io.atcr.sailor.star","subject":{"did":"did:plc:alice123","repository":"myapp"},"createdAt":"2025-01-01T00:00:00Z"}`,
-			wantSubject:  "at://did:plc:alice123/io.atcr.repo.page/myapp",
-			wantOwnerDID: "did:plc:alice123",
-			wantRepo:     "myapp",
-		},
+func TestStarRecord_Deserialization(t *testing.T) {
+	const jsonData = `{"$type":"io.atcr.sailor.star","subject":"at://did:plc:alice123/io.atcr.repo.page/myapp","createdAt":"2025-01-01T00:00:00Z"}`
+
+	var record StarRecord
+	if err := json.Unmarshal([]byte(jsonData), &record); err != nil {
+		t.Fatalf("Unmarshal error: %v", err)
 	}

-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			var record StarRecord
-			if err := json.Unmarshal([]byte(tt.jsonData), &record); err != nil {
-				t.Fatalf("Unmarshal error: %v", err)
-			}
+	if record.Subject != "at://did:plc:alice123/io.atcr.repo.page/myapp" {
+		t.Errorf("Subject = %v", record.Subject)
+	}

-			if record.Subject != tt.wantSubject {
-				t.Errorf("Subject = %v, want %v", record.Subject, tt.wantSubject)
-			}
-
-			ownerDID, repo, err := record.GetSubjectDIDAndRepository()
-			if err != nil {
-				t.Fatalf("GetSubjectDIDAndRepository error: %v", err)
-			}
-			if ownerDID != tt.wantOwnerDID {
-				t.Errorf("ownerDID = %v, want %v", ownerDID, tt.wantOwnerDID)
-			}
-			if repo != tt.wantRepo {
-				t.Errorf("repository = %v, want %v", repo, tt.wantRepo)
-			}
-		})
+	ownerDID, repo, err := record.GetSubjectDIDAndRepository()
+	if err != nil {
+		t.Fatalf("GetSubjectDIDAndRepository error: %v", err)
+	}
+	if ownerDID != "did:plc:alice123" {
+		t.Errorf("ownerDID = %v", ownerDID)
+	}
+	if repo != "myapp" {
+		t.Errorf("repository = %v", repo)
 	}
 }

--- a/pkg/auth/hold_authorizer.go
+++ b/pkg/auth/hold_authorizer.go
@@ -30,6 +30,16 @@ type HoldAuthorizer interface {
 	// Called when user successfully becomes a crew member to ensure immediate access
 	// Returns nil if no denial cache exists or invalidation succeeds
 	ClearCrewDenial(ctx context.Context, holdDID, userDID string) error
+
+	// IsCachedCrewMember returns true only if there is a non-expired approval
+	// in the cache. It MUST NOT make any network calls. Cache miss returns (false, nil).
+	IsCachedCrewMember(ctx context.Context, holdDID, userDID string) (bool, error)
+
+	// RecordCrewApproval writes an approval to the cache with the implementation's
+	// standard TTL. Used to warm the cache after an out-of-band confirmation of crew
+	// membership (e.g. a successful requestCrew POST). No-op for implementations
+	// without a cache.
+	RecordCrewApproval(ctx context.Context, holdDID, userDID string) error
 }

 // CheckReadAccessWithCaptain implements the standard read authorization logic
--- a/pkg/auth/hold_remote.go
+++ b/pkg/auth/hold_remote.go
@@ -401,6 +401,24 @@ func (a *RemoteHoldAuthorizer) isCrewMemberNoCache(ctx context.Context, holdDID,
 	return false, nil
 }

+// IsCachedCrewMember returns true if there is a non-expired approval row.
+// Never makes network calls. Cache miss or no DB returns (false, nil).
+func (a *RemoteHoldAuthorizer) IsCachedCrewMember(ctx context.Context, holdDID, userDID string) (bool, error) {
+	if a.db == nil {
+		return false, nil
+	}
+	return a.getCachedApproval(holdDID, userDID)
+}
+
+// RecordCrewApproval writes an approval to the cache with the standard 15-min TTL.
+// No-op if there is no DB.
+func (a *RemoteHoldAuthorizer) RecordCrewApproval(ctx context.Context, holdDID, userDID string) error {
+	if a.db == nil {
+		return nil
+	}
+	return a.cacheApproval(holdDID, userDID, 15*time.Minute)
+}
+
 // CheckReadAccess implements read authorization using shared logic
 func (a *RemoteHoldAuthorizer) CheckReadAccess(ctx context.Context, holdDID, userDID string) (bool, error) {
 	captain, err := a.GetCaptainRecord(ctx, holdDID)
--- a/pkg/auth/hold_remote_test.go
+++ b/pkg/auth/hold_remote_test.go
@@ -446,3 +446,129 @@ func TestClearAllDenials_OnStartup(t *testing.T) {
 		t.Error("Expected all denials to be cleared after ClearAllDenials")
 	}
 }
+
+func TestIsCachedCrewMember_Hit(t *testing.T) {
+	testDB := setupTestDB(t)
+	remote := NewRemoteHoldAuthorizer(testDB, false).(*RemoteHoldAuthorizer)
+	defer close(remote.stopCleanup)
+
+	holdDID := "did:web:hold01.atcr.io"
+	userDID := "did:plc:user123"
+
+	if err := remote.cacheApproval(holdDID, userDID, 15*time.Minute); err != nil {
+		t.Fatalf("cacheApproval failed: %v", err)
+	}
+
+	cached, err := remote.IsCachedCrewMember(context.Background(), holdDID, userDID)
+	if err != nil {
+		t.Fatalf("IsCachedCrewMember returned error: %v", err)
+	}
+	if !cached {
+		t.Error("Expected cache hit, got miss")
+	}
+}
+
+func TestIsCachedCrewMember_Miss(t *testing.T) {
+	testDB := setupTestDB(t)
+	remote := NewRemoteHoldAuthorizer(testDB, false).(*RemoteHoldAuthorizer)
+	defer close(remote.stopCleanup)
+
+	cached, err := remote.IsCachedCrewMember(context.Background(),
+		"did:web:hold01.atcr.io", "did:plc:nobody")
+	if err != nil {
+		t.Fatalf("IsCachedCrewMember returned error: %v", err)
+	}
+	if cached {
+		t.Error("Expected cache miss, got hit")
+	}
+}
+
+func TestIsCachedCrewMember_Expired(t *testing.T) {
+	testDB := setupTestDB(t)
+	remote := NewRemoteHoldAuthorizer(testDB, false).(*RemoteHoldAuthorizer)
+	defer close(remote.stopCleanup)
+
+	holdDID := "did:web:hold01.atcr.io"
+	userDID := "did:plc:user123"
+
+	// Insert a row that already expired one minute ago.
+	now := time.Now()
+	_, err := testDB.Exec(`
+		INSERT INTO hold_crew_approvals (hold_did, user_did, approved_at, expires_at)
+		VALUES (?, ?, ?, ?)
+	`, holdDID, userDID, now.Add(-2*time.Minute), now.Add(-1*time.Minute))
+	if err != nil {
+		t.Fatalf("seed insert failed: %v", err)
+	}
+
+	cached, err := remote.IsCachedCrewMember(context.Background(), holdDID, userDID)
+	if err != nil {
+		t.Fatalf("IsCachedCrewMember returned error: %v", err)
+	}
+	if cached {
+		t.Error("Expected expired entry to be treated as miss")
+	}
+
+	// Expired entry should have been cleaned up by getCachedApproval.
+	var count int
+	if err := testDB.QueryRow(`
+		SELECT COUNT(*) FROM hold_crew_approvals WHERE hold_did = ? AND user_did = ?
+	`, holdDID, userDID).Scan(&count); err != nil {
+		t.Fatalf("count query failed: %v", err)
+	}
+	if count != 0 {
+		t.Errorf("Expected expired row to be deleted, found %d", count)
+	}
+}
+
+func TestRecordCrewApproval_WritesAndReadsBack(t *testing.T) {
+	testDB := setupTestDB(t)
+	remote := NewRemoteHoldAuthorizer(testDB, false).(*RemoteHoldAuthorizer)
+	defer close(remote.stopCleanup)
+
+	holdDID := "did:web:hold01.atcr.io"
+	userDID := "did:plc:user123"
+
+	if err := remote.RecordCrewApproval(context.Background(), holdDID, userDID); err != nil {
+		t.Fatalf("RecordCrewApproval failed: %v", err)
+	}
+
+	cached, err := remote.IsCachedCrewMember(context.Background(), holdDID, userDID)
+	if err != nil {
+		t.Fatalf("IsCachedCrewMember returned error: %v", err)
+	}
+	if !cached {
+		t.Error("Expected RecordCrewApproval to populate the cache")
+	}
+
+	// Verify TTL is roughly 15 minutes from now.
+	var expiresAt time.Time
+	if err := testDB.QueryRow(`
+		SELECT expires_at FROM hold_crew_approvals WHERE hold_did = ? AND user_did = ?
+	`, holdDID, userDID).Scan(&expiresAt); err != nil {
+		t.Fatalf("expires_at query failed: %v", err)
+	}
+	ttl := time.Until(expiresAt)
+	if ttl < 14*time.Minute || ttl > 16*time.Minute {
+		t.Errorf("Expected TTL ~15min, got %v", ttl)
+	}
+}
+
+func TestIsCachedCrewMember_NoDB(t *testing.T) {
+	remote := NewRemoteHoldAuthorizer(nil, false).(*RemoteHoldAuthorizer)
+	defer close(remote.stopCleanup)
+
+	cached, err := remote.IsCachedCrewMember(context.Background(),
+		"did:web:hold01.atcr.io", "did:plc:user123")
+	if err != nil {
+		t.Errorf("Expected nil error with nil DB, got %v", err)
+	}
+	if cached {
+		t.Error("Expected false with nil DB")
+	}
+
+	if err := remote.RecordCrewApproval(context.Background(),
+		"did:web:hold01.atcr.io", "did:plc:user123"); err != nil {
+		t.Errorf("Expected nil error from RecordCrewApproval with nil DB, got %v", err)
+	}
+}
--- a/pkg/auth/holdlocal/holdlocal.go
+++ b/pkg/auth/holdlocal/holdlocal.go
@@ -102,3 +102,14 @@ func (a *Authorizer) CheckWriteAccess(ctx context.Context, holdDID, userDID stri
 func (a *Authorizer) ClearCrewDenial(ctx context.Context, holdDID, userDID string) error {
 	return nil
 }
+
+// IsCachedCrewMember always returns (false, nil) for the local authorizer.
+// There is no cache; callers fall through to the direct PDS lookup.
+func (a *Authorizer) IsCachedCrewMember(ctx context.Context, holdDID, userDID string) (bool, error) {
+	return false, nil
+}
+
+// RecordCrewApproval is a no-op for the local authorizer (no cache to warm).
+func (a *Authorizer) RecordCrewApproval(ctx context.Context, holdDID, userDID string) error {
+	return nil
+}
--- a/pkg/hold/pds/crew.go
+++ b/pkg/hold/pds/crew.go
@@ -5,7 +5,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log/slog"
 	"strings"
 	"time"

@@ -202,95 +201,3 @@ func (p *HoldPDS) UpdateCrewMemberTier(ctx context.Context, memberDID, tier stri

 	return nil
 }
-
-// TODO(crew-migration): Remove this migration code after all holds have been upgraded (added 2026-01-06)
-// This migrates TID-based crew records to hash-based rkeys for O(1) lookups
-
-// MigrateCrewRecordsToHashRkeys migrates old TID-based crew records to hash-based rkeys
-// This is idempotent - records that already have hash-based rkeys are skipped
-// Returns the number of records migrated
-func (p *HoldPDS) MigrateCrewRecordsToHashRkeys(ctx context.Context) (int, error) {
-	// List all crew members (includes both TID and hash-based rkeys)
-	members, err := p.ListCrewMembers(ctx)
-	if err != nil {
-		return 0, fmt.Errorf("failed to list crew members: %w", err)
-	}
-
-	slog.Info("Starting crew record migration", "totalRecords", len(members))
-
-	migrated := 0
-	duplicatesDeleted := 0
-	alreadyHashBased := 0
-	seen := make(map[string]bool) // Track seen member DIDs to handle duplicates
-
-	for _, m := range members {
-		memberDID := m.Record.Member
-		expectedRkey := atproto.CrewRecordKey(memberDID)
-
-		// Skip if already using hash-based rkey
-		if m.Rkey == expectedRkey {
-			seen[memberDID] = true
-			alreadyHashBased++
-			continue
-		}
-
-		// This is a TID-based record that needs migration
-		slog.Info("Migrating crew record to hash-based rkey",
-			"memberDID", memberDID,
-			"oldRkey", m.Rkey,
-			"newRkey", expectedRkey)
-
-		// Check if we already have a hash-based record for this DID (duplicate handling)
-		if seen[memberDID] {
-			// Already migrated this DID, just delete the old TID record
-			slog.Info("Deleting duplicate TID-based crew record",
-				"memberDID", memberDID,
-				"rkey", m.Rkey)
-			if err := p.RemoveCrewMember(ctx, m.Rkey); err != nil {
-				slog.Warn("Failed to delete duplicate crew record",
-					"rkey", m.Rkey,
-					"error", err)
-			} else {
-				duplicatesDeleted++
-			}
-			continue
-		}
-
-		// Create new record with hash-based rkey (PutRecord handles upsert)
-		newRecord := &atproto.CrewRecord{
-			Type:        atproto.CrewCollection,
-			Member:      m.Record.Member,
-			Role:        m.Record.Role,
-			Permissions: m.Record.Permissions,
-			Tier:        m.Record.Tier,
-			AddedAt:     m.Record.AddedAt,
-		}
-
-		_, _, err := p.repomgr.PutRecord(ctx, p.uid, atproto.CrewCollection, expectedRkey, newRecord)
-		if err != nil {
-			slog.Error("Failed to create hash-based crew record",
-				"memberDID", memberDID,
-				"error", err)
-			continue
-		}
-
-		// Delete the old TID-based record
-		if err := p.RemoveCrewMember(ctx, m.Rkey); err != nil {
-			slog.Warn("Failed to delete old TID-based crew record",
-				"rkey", m.Rkey,
-				"error", err)
-			// Continue anyway - the new record is created
-		}
-
-		seen[memberDID] = true
-		migrated++
-	}
-
-	slog.Info("Crew record migration complete",
-		"migrated", migrated,
-		"duplicatesDeleted", duplicatesDeleted,
-		"alreadyHashBased", alreadyHashBased,
-		"totalRecords", len(members))
-
-	return migrated, nil
-}
--- a/pkg/hold/pds/hold_pds.go
+++ b/pkg/hold/pds/hold_pds.go
@@ -353,14 +353,6 @@ func (p *HoldPDS) Bootstrap(ctx context.Context, s3svc *s3.S3Service, cfg Bootst
 		}
 	}

-	// TODO(crew-migration): Remove this call after all holds have been upgraded (added 2026-01-06)
-	// Migrate TID-based crew records to hash-based rkeys for O(1) lookups
-	if migrated, err := p.MigrateCrewRecordsToHashRkeys(ctx); err != nil {
-		slog.Warn("Crew record migration failed", "error", err)
-	} else if migrated > 0 {
-		slog.Info("Migrated crew records to hash-based rkeys", "count", migrated)
-	}
-
 	// Create or sync Bluesky profile record from config
 	// This runs even if captain exists (for existing holds being upgraded)
 	// Skip if no S3 service (e.g., in tests)