From 1461273162d82f0a28853870f7d6b1c706a98e78 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Mon, 10 Aug 2026 10:43:05 -0400 Subject: [PATCH] fix(sync): wire dead token cleanup queries into daily maintenance runner CleanupExpiredRefreshTokens and CleanupExpiredOpdsTokens were generated by sqlc but never invoked anywhere in the codebase, so expired/revoked tokens accumulated in the database indefinitely. The refresh-token query was parameterized in the settings-registry work specifically so its retention window could follow the configurable session duration, but the periodic caller was never wired up. annotations.go: - Rename StartTombstonePurger to StartDailyMaintenance, which now runs all periodic cleanup tasks from a single 24h-tick goroutine. - Add runDailyMaintenance helper: tombstones, then OPDS tokens, then refresh tokens, each logging independently so one failure never skips the others. - Refresh-token retention is read from the registry (SessionDuration) on every tick so live admin edits are honored; guarded on the registry being wired so unwired test paths simply skip cleanup. - All three queries only delete rows that are already expired or revoked, so active sessions are never logged out. main.go: - Update the call site: tombstonePurgerCancel becomes maintenanceCancel and calls StartDailyMaintenance. Net footprint: still one goroutine and one ticker; the cleanup adds one DELETE per table per day. --- cmd/server/main.go | 4 ++-- internal/sync/annotations.go | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index cde17fd..829173b 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -111,8 +111,8 @@ func main() { progressService := sync.NewProgressService(queries, connManager) annotationService := sync.NewAnnotationService(queries, connManager) annotationService.SetSettings(registry) - tombstonePurgerCancel := annotationService.StartTombstonePurger() - defer tombstonePurgerCancel() + maintenanceCancel := annotationService.StartDailyMaintenance() + defer maintenanceCancel() queueProcessor := sync.NewSyncQueueProcessorWithConfig(queries, registry.SyncQueueConfig().Interval, registry.SyncQueueConfig().BatchSize) queueProcessor.SetProgressService(progressService) diff --git a/internal/sync/annotations.go b/internal/sync/annotations.go index 4689436..f30b353 100644 --- a/internal/sync/annotations.go +++ b/internal/sync/annotations.go @@ -272,7 +272,13 @@ func (s *AnnotationService) PurgeExpiredTombstones(ctx context.Context) error { return nil } -func (s *AnnotationService) StartTombstonePurger() context.CancelFunc { +// StartDailyMaintenance launches a single background goroutine that runs all +// periodic cleanup tasks once every 24 hours: expired annotation tombstones, +// expired/revoked refresh tokens (retention follows the configured session +// duration), and expired OPDS tokens. Each task is independent; a failure in +// one is logged and does not skip the others. The returned CancelFunc stops the +// goroutine and the underlying ticker; it must be invoked on shutdown. +func (s *AnnotationService) StartDailyMaintenance() context.CancelFunc { ticker := time.NewTicker(24 * time.Hour) ctx, cancel := context.WithCancel(context.Background()) @@ -283,9 +289,7 @@ func (s *AnnotationService) StartTombstonePurger() context.CancelFunc { ticker.Stop() return case <-ticker.C: - if err := s.PurgeExpiredTombstones(ctx); err != nil { - log.Printf("AnnotationService: tombstone purge failed: %v", err) - } + s.runDailyMaintenance(ctx) } } }() @@ -293,6 +297,28 @@ func (s *AnnotationService) StartTombstonePurger() context.CancelFunc { return cancel } +// runDailyMaintenance executes every periodic cleanup task. Tasks run +// sequentially under the single daily-tick goroutine so there is no added +// concurrency. All three queries only delete rows that are already unusable +// (expired or revoked), so this never logs out active sessions. +func (s *AnnotationService) runDailyMaintenance(ctx context.Context) { + if err := s.PurgeExpiredTombstones(ctx); err != nil { + log.Printf("maintenance: tombstone purge failed: %v", err) + } + if err := s.db.CleanupExpiredOpdsTokens(ctx); err != nil { + log.Printf("maintenance: OPDS token purge failed: %v", err) + } + // Refresh-token retention follows the configured session duration; re-read + // on every tick so live settings changes are honored. Guarded so unwired + // test paths simply skip cleanup (production always wires the registry). + if s.settings != nil { + retention := s.settings.SessionDuration().Seconds() + if err := s.db.CleanupExpiredRefreshTokens(ctx, retention); err != nil { + log.Printf("maintenance: refresh token purge failed: %v", err) + } + } +} + type SaveNoteRequest struct { MediaItemID pgtype.UUID UserID pgtype.UUID