auth: stop wiping the shared denial cache on every boot

ClearAllDenials ran unconditionally at startup, and its database half is
"DELETE FROM hold_crew_denials" with no scoping at all. One instance, that is a
clean slate on deploy. Several instances, and a rolling deploy wipes the shared
table once per instance while every scale-out event wipes it again, so the
backoff that exists to stop a denied client hammering a hold's PDS keeps getting
reset out from under it.

The intent is worth keeping: a restart usually means a fix shipped, and someone
sitting on a backoff of up to an hour should get to retry rather than wait it
out. So it moves under the cleanup lease instead of being deleted, and now
happens once per deploy rather than once per instance.

Worth noting the in-memory half was always a no-op here. recentDenials belongs
to the process, and a process that has just started has an empty one, so the
table-wide DELETE was the only thing the startup call ever really did.

The cleanup worker moved down past the hold authorizer's construction, since it
now needs a handle on it. Reading s.HoldAuthorizer from the worker goroutine
while the constructor was still assigning it would have been a data race.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/appview/server.go

@@ -201,11 +201,8 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,
 	slog.Info("Lease manager initialized",
 		"component", "leases", "enabled", cfg.Leases.Enabled, "holder", s.Leases.HolderID())
 
-	// Session, OAuth and device-flow cleanup. Idempotent DELETEs, so running it
-	// on every instance would be harmless but wasteful; more importantly it
-	// belongs with the other leased workers rather than buried in the database
-	// constructor, where it had no way to reach the lease manager.
-	s.startCleanupWorker(workerCtx)
+	// The cleanup worker starts later, once the hold authorizer exists: it
+	// clears the crew denial backoffs as its first act and needs a handle on it.
 
 	// Initialize OAuth components
 	slog.Info("Initializing OAuth components")
@@ -271,14 +268,10 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer,
 	middleware.SetGlobalAuthorizer(s.HoldAuthorizer)
 	slog.Info("Hold authorizer initialized with database caching")
 
-	// Clear all denial caches on startup for a clean slate
-	if remote, ok := s.HoldAuthorizer.(*auth.RemoteHoldAuthorizer); ok {
-		go func() {
-			if err := remote.ClearAllDenials(); err != nil {
-				slog.Warn("Failed to clear denial caches on startup", "error", err)
-			}
-		}()
-	}
+	// Session, OAuth and device-flow cleanup, plus the one-shot denial-cache
+	// clear. Started here rather than next to the other workers because it needs
+	// the hold authorizer above.
+	s.startCleanupWorker(workerCtx)
 
 	// Initialize billing manager
 	appviewDID := DIDFromBaseURL(baseURL)
@@ -982,10 +975,33 @@ func (s *AppViewServer) waitForLeasedWorkers() {
 	s.Leases.Wait(leaseDrainTimeout)
 }
 
-// startCleanupWorker runs the periodic expiry sweep under the cleanup lease.
+// startCleanupWorker runs the periodic expiry sweep under the cleanup lease,
+// after clearing the crew denial backoffs once on acquiring it.
+//
+// That clear used to run on every boot, unconditionally, as
+// "DELETE FROM hold_crew_denials" with no scoping. The intent is sound: a
+// restart usually means a fix has shipped, and users sitting on a backoff of up
+// to an hour should get to retry immediately rather than wait it out. The
+// implementation stopped being sound the moment there was more than one
+// instance, because then a rolling deploy wipes the shared table once per
+// instance and every scale-out event wipes it again.
+//
+// Moving it under the lease keeps the behavior (a deploy still clears the
+// backoffs) while making it happen once. Note the in-memory half of
+// ClearAllDenials was always a no-op at startup, since a fresh process has an
+// empty map; the database wipe was the only thing it ever really did.
 func (s *AppViewServer) startCleanupWorker(ctx context.Context) {
 	const cleanupInterval = time.Hour
+	authorizer := s.HoldAuthorizer
+
 	s.Leases.Go(ctx, db.LeaseCleanup, func(leaseCtx context.Context) error {
+		if remote, ok := authorizer.(*auth.RemoteHoldAuthorizer); ok {
+			if err := remote.ClearAllDenials(); err != nil {
+				// Not fatal: the denials carry their own next_retry_at and will
+				// lapse on their own.
+				slog.Warn("Failed to clear denial caches", "error", err)
+			}
+		}
 		return db.RunPeriodicCleanup(leaseCtx, s.Database, s.SessionStore, cleanupInterval)
 	})
 }

pkg/auth/hold_remote.go

@@ -674,8 +674,15 @@ func (a *RemoteHoldAuthorizer) ClearCrewDenial(ctx context.Context, holdDID, use
 	return nil
 }
 
-// ClearAllDenials removes all crew denials from both in-memory and database caches
-// Called on startup to ensure a clean slate
+// ClearAllDenials removes all crew denials from both in-memory and database
+// caches, giving users sitting on a long backoff an immediate retry after a
+// deploy.
+//
+// Called by the AppView's cleanup worker when it acquires the cleanup lease, NOT
+// on every boot. The database half is a table-wide DELETE shared by every
+// instance, so calling it unconditionally at startup meant a rolling deploy
+// wiped the backoffs once per instance and every scale-out event wiped them
+// again. Keep it behind the lease.
 func (a *RemoteHoldAuthorizer) ClearAllDenials() error {
 	// Clear all in-memory denials
 	a.recentDenials.Range(func(key, value any) bool {