# Plan: Consolidated `jobs` Package for Background Cron Tasks

**Goal:** Extract 9 side-effect cleanup functions from `GetAvailableHours` HTTP handler into a dedicated `jobs` package with cron-scheduled, parallel execution. Remove the lazy-cleanup antipattern.

**Current state:**
- 9 cleanup functions in `backend/handlers/scheduling/time-blockers.go` — all called synchronously in `default-hours.go:364-407` on every `GET /available-hours` request
- Only `CleanupOldReservations` ALSO runs on a background goroutine (`main.go:432-450`, 5min ticker)
- 4 other ad-hoc background goroutines exist (JWT cleanup, GDPR cache, login state, rate limiter)
- `robfig/cron/v3` v3.0.1 is already a `go.mod` dependency (currently used only for parsing time-blocker cron expressions)

---

## Phase 1: Create `backend/internal/jobs/` Package

**New package:** `crussell/internal/jobs`

### File: `scheduler.go`

Core scheduler abstraction built on `robfig/cron/v3`:

```go
package jobs

import (
    "context"
    "log"
    "time"
    "github.com/robfig/cron/v3"
)

// Job defines a single periodic task.
type Job struct {
    Name        string                         // Human-readable name (for logging)
    Schedule    string                         // Standard cron expression ("*/5 * * * *")
    Timeout     time.Duration                  // Per-execution timeout
    Concurrency int                            // Max concurrent runs (0 = unlimited, 1 = serial)
    Handler     func(context.Context) error     // The actual work
}

// Scheduler manages all registered cron jobs with parallel execution.
type Scheduler struct {
    cron       *cron.Cron
    entries    []cron.EntryID
    registry   []Job
    baseCtx    context.Context
    cancel     context.CancelFunc
    semaphores map[string]chan struct{}     // Per-job concurrency limit
}
```

**Key design decisions:**
1. Each job runs in its own goroutine (cron v3 default) — parallel by nature
2. Per-job concurrency control via channel semaphore — prevents overlapping runs of the same job
3. Panic recovery wrapper — matches existing pattern in `jwt.go`, `gdpr_export.go`, `auth/local.go`
4. Timeout via `context.WithTimeout` — matches existing `main.go` pattern
5. Graceful shutdown via `baseCtx` — the cron scheduler is stopped, then active jobs drain

```go
func New() *Scheduler {
    ctx, cancel := context.WithCancel(context.Background())
    return &Scheduler{
        cron:       cron.New(cron.WithLocation(londonLocation)),
        baseCtx:    ctx,
        cancel:     cancel,
        semaphores: make(map[string]chan struct{}),
    }
}

func (s *Scheduler) Register(job Job) {
    // Validate cron expression at registration time
    parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
    if _, err := parser.Parse(job.Schedule); err != nil {
        log.Fatalf("jobs: invalid cron schedule %q for job %q: %v", job.Schedule, job.Name, err)
    }
    s.registry = append(s.registry, job)
}

func (s *Scheduler) Start() {
    for _, job := range s.registry {
        j := job // capture
        entryID, err := s.cron.AddFunc(j.Schedule, s.wrapJob(j))
        if err != nil {
            log.Fatalf("jobs: failed to register %q: %v", j.Name, err)
        }
        s.entries = append(s.entries, entryID)
    }
    s.cron.Start()
}

func (s *Scheduler) Shutdown() <-chan struct{} {
    ctx := s.cron.Stop() // Stop scheduler (returns ctx that completes when all jobs finish)
    s.cancel()           // Cancel base context so in-flight jobs know to stop
    return ctx.Done()    // Caller can wait for this
}

// wrapJob adds panic recovery, timeout, concurrency control, and logging.
func (s *Scheduler) wrapJob(job Job) func() {
    // Set up semaphore if concurrency limited
    var sem chan struct{}
    if job.Concurrency > 0 {
        sem = make(chan struct{}, job.Concurrency)
        sem <- struct{}{} // Initial slot filled
    }

    return func() {
        // Concurrency guard
        if sem != nil {
            select {
            case <-sem:
                // Acquired — proceed
            default:
                log.Printf("jobs: %q skipped (previous run still in progress)", job.Name)
                return
            }
            defer func() { sem <- struct{}{} }()
        }

        // Panic recovery
        defer func() {
            if r := recover(); r != nil {
                log.Printf("jobs: panic recovered in %q: %v", job.Name, r)
            }
        }()

        // Timeout
        ctx, cancel := context.WithTimeout(s.baseCtx, job.Timeout)
        defer cancel()

        start := time.Now()
        if err := job.Handler(ctx); err != nil {
            log.Printf("jobs: %q failed: %v (duration: %v)", job.Name, err, time.Since(start))
        } else {
            log.Printf("jobs: %q completed (duration: %v)", job.Name, time.Since(start))
        }
    }
}
```

### File: `cleanup.go`

Registration of all 9 scheduling cleanup functions + JWT cleanup.

```go
package jobs

import (
    "context"
    "crussell/auth"
    "crussell/handlers/scheduling"
    "time"
)

// RegisterAll registers every background job.
func RegisterAll(s *Scheduler) {
    // === HIGH FREQUENCY (every 5 min) ===

    s.Register(Job{
        Name:        "cleanup-reservations",
        Schedule:    "*/5 * * * *",
        Timeout:     30 * time.Second,
        Concurrency: 1,
        Handler:     scheduling.CleanupOldReservations,
    })

    s.Register(Job{
        Name:        "cleanup-expired-deposits",
        Schedule:    "*/5 * * * *",
        Timeout:     30 * time.Second,
        Concurrency: 1,
        Handler:     scheduling.CleanupExpiredDeposits,
    })

    // === MID FREQUENCY (hourly) ===

    s.Register(Job{
        Name:        "cleanup-expired-loyalty-redemptions",
        Schedule:    "0 * * * *",
        Timeout:     30 * time.Second,
        Concurrency: 1,
        Handler:     scheduling.CleanupExpiredLoyaltyRedemptions,
    })

    s.Register(Job{
        Name:        "cleanup-old-idempotency-keys",
        Schedule:    "0 * * * *",
        Timeout:     30 * time.Second,
        Concurrency: 1,
        Handler:     scheduling.CleanupOldIdempotencyKeys,
    })

    s.Register(Job{
        Name:        "cleanup-revoked-jtis",
        Schedule:    "0 * * * *",
        Timeout:     30 * time.Second,
        Concurrency: 1,
        Handler: func(ctx context.Context) error {
            auth.CleanupRevokedJTIs(ctx)
            return nil
        },
    })

    // === LOW FREQUENCY (daily, off-peak) ===

    s.Register(Job{
        Name:        "anonymize-stale-guest-accounts",
        Schedule:    "0 3 * * *",           // 3am
        Timeout:     5 * time.Minute,
        Concurrency: 1,
        Handler:     scheduling.AnonymizeStaleGuestAccounts,
    })

    s.Register(Job{
        Name:        "cleanup-expired-financial-records",
        Schedule:    "0 4 * * *",           // 4am
        Timeout:     10 * time.Minute,
        Concurrency: 1,
        Handler:     scheduling.CleanupExpiredFinancialRecords,
    })

    s.Register(Job{
        Name:        "cleanup-expired-gift-cards",
        Schedule:    "0 5 * * *",           // 5am
        Timeout:     5 * time.Minute,
        Concurrency: 1,
        Handler:     scheduling.CleanupExpiredGiftCards,
    })

    s.Register(Job{
        Name:        "cleanup-idle-accounts",
        Schedule:    "30 3 * * *",          // 3:30am (offset from financial)
        Timeout:     5 * time.Minute,
        Concurrency: 1,
        Handler:     scheduling.CleanupIdleAccounts,
    })

    s.Register(Job{
        Name:        "cleanup-old-name-history",
        Schedule:    "30 4 * * *",          // 4:30am
        Timeout:     30 * time.Second,
        Concurrency: 1,
        Handler:     scheduling.CleanupOldNameHistory,
    })
}
```

**Staggering rationale:** Daily jobs at 3am/3:30am/4am/4:30am/5am spread the load so heavy operations (financial aggregation, idle account scans) don't contend with each other or with the hourly jobs. High-frequency jobs (reservations, deposits) are decoupled on their own 5-min schedules.

---

## Phase 2: Wire into `main.go`

### Before:
```go
// main.go:142
auth.StartJTICleanup()

// main.go:432-450 (background goroutine for reservations)
cleanupCtx, cleanupStop := context.WithCancel(context.Background())
go func() {
    ticker := time.NewTicker(reservationCleanupInterval)
    ...
}()

// main.go:457-468 (shutdown)
cleanupStop()
```

### After:
```go
// At package level, after constants but before init()
var sched *jobs.Scheduler

// In main(), after initDB()/initSquare()/etc but before r := chi.NewRouter():
sched = jobs.New()
jobs.RegisterAll(sched)
sched.Start()

// In main(), shutdown block (replace the old cleanupStop() call):
quit := make(chan os.Signal, 1)
signal.Notify(quit, syscall.SIGTERM, syscall.SIGINT)
go func() {
    <-quit
    log.Println("Shutting down server...")
    
    // Graceful HTTP shutdown
    ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
    defer cancel()
    if err := srv.Shutdown(ctx); err != nil {
        log.Printf("Server forced to shutdown: %v", err)
    }
    
    // Graceful job scheduler shutdown
    <-sched.Shutdown()
    log.Println("Background jobs stopped")
}()
```

**Remove:**
- `reservationCleanupInterval` and `reservationCleanupTimeout` constants (`main.go:71-72`)
- `cleanupCtx`/`cleanupStop` variables (`main.go:433`)
- The goroutine block (`main.go:434-450`)
- Import `scheduling` from `main.go` (no longer needed there)
- `auth.StartJTICleanup()` call (`main.go:142`)

---

## Phase 3: Remove Side-Effects from `GetAvailableHours`

**File: `backend/handlers/scheduling/default-hours.go`**

Delete lines 364-407 (the entire cleanup block):

```diff
-	// Clean up old reservations (older than 1 hour)
-	if err := CleanupOldReservations(r.Context()); err != nil {
-		log.Printf("Failed to cleanup old reservations: %v", err)
-	}
-
-	// Anonymize stale guest accounts (6+ months after last booking)
-	if err := AnonymizeStaleGuestAccounts(r.Context()); err != nil {
-		log.Printf("Failed to anonymize stale guest accounts: %v", err)
-	}
-
-	// Clean up expired loyalty redemptions (pending past expires_at)
-	if err := CleanupExpiredLoyaltyRedemptions(r.Context()); err != nil {
-		log.Printf("Failed to cleanup expired loyalty redemptions: %v", err)
-	}
-
-	// Clean up expired financial records (aggregate + delete granular data)
-	if err := CleanupExpiredFinancialRecords(r.Context()); err != nil {
-		log.Printf("Failed to cleanup expired financial records: %v", err)
-	}
-
-	// Clean up bookings past deposit deadline (no deposit paid)
-	if err := CleanupExpiredDeposits(r.Context()); err != nil {
-		log.Printf("Failed to cleanup expired deposits: %v", err)
-	}
-
-	// Clean up expired gift cards (unused for 24+ months)
-	if err := CleanupExpiredGiftCards(r.Context()); err != nil {
-		log.Printf("Failed to cleanup expired gift cards: %v", err)
-	}
-
-	// Clean up idle accounts (2yr no money, 5yr with money)
-	if err := CleanupIdleAccounts(r.Context()); err != nil {
-		log.Printf("Failed to cleanup idle accounts: %v", err)
-	}
-
-	// Clean up old idempotency keys (24h+ and non-pending)
-	if err := CleanupOldIdempotencyKeys(r.Context()); err != nil {
-		log.Printf("Failed to cleanup old idempotency keys: %v", err)
-	}
-
-	// Clean up old name history (6+ months)
-	if err := CleanupOldNameHistory(r.Context()); err != nil {
-		log.Printf("Failed to cleanup old name history: %v", err)
-	}
```

Also remove `"log"` import from `default-hours.go` if it becomes unused after removal.

---

## Phase 4: Remove `auth.StartJTICleanup()` (Optional but Recommended)

**File: `backend/auth/jwt.go`**

- Keep `CleanupRevokedJTIs(ctx)` — it's used by tests and now by the jobs package
- Delete `StartJTICleanup()` entirely (no longer needed, replaced by the jobs package)
- Remove the ticker goroutine

---

## Phase 5: Update Documentation

**File: `obsidian/Crussell/Technical Manual.md`**
- Update lines ~545 and ~999: change "lazy cleanup triggered on availability fetch" to "all cleanup runs on cron schedules via the `jobs` package"
- Remove the lazy-cleanup justification
- Add a new section documenting the `jobs` package and its job registry

**File: `obsidian/Crussell/Future Work - Gap Backlog.md`**
- Mark item #2 as fully complete (strikethrough)
- Remove the note about remaining functions still running on `GET /api/availability`

**File: `README.md`** (optional)
- Add a line about the background job scheduler

---

## Effort Estimate

| Step | Files Changed | Effort |
|------|--------------|--------|
| Phase 1a: `scheduler.go` | 1 new file | 2-3h |
| Phase 1b: `cleanup.go` | 1 new file | 1h |
| Phase 2: Wire `main.go` | 1 file | 30min |
| Phase 3: Remove side-effects from handler | 1 file | 15min |
| Phase 4: Remove `StartJTICleanup` | 1 file | 15min |
| Phase 5: Update docs | 2-3 files | 30min |
| Testing & go vet | — | 1h |
| **Total** | **4-5 files (+2 new)** | **~6h** |

---

## Risks & Mitigations

| Risk | Mitigation |
|------|------------|
| **Concurrent DB load** from multiple jobs at the same cron tick | Stagger daily jobs across 3am-5am range. High-frequency jobs (5min) are lightweight. Per-job `Concurrency: 1` prevents overlapping runs. |
| **Job takes longer than interval** (e.g., financial aggregation >5min while scheduled every 5min) | `Concurrency: 1` + skip-logic: if previous run still in-flight, the new invocation is skipped and logged. |
| **Cron expression parsing differs** from existing time-blocker parser | Both use `cron.NewParser(cron.Minute \| cron.Hour \| cron.Dom \| cron.Month \| cron.Dow)` — identical. |
| **Tests expect cleanup side-effects** from `GetAvailableHours` | Search for tests that rely on the side-effect calls. Some may need an explicit cleanup call before assertions. |
| **`CleanupExpiredDeposits` timing gap** — deposit-lapsed slots not freed until next cron tick | 5-min frequency is acceptable. The previous behavior freed on the *next* availability fetch, which could be minutes or hours apart depending on user activity. 5-min max latency is actually *better* than the lazy approach during quiet periods. |
| **Panic in one job takes down the scheduler** | `wrapJob` has defer-recover. One job's panic cannot affect others (separate goroutines). |

---

## Edge Cases & Exclusions

**Not in scope (Phase 1):**
- `gdpr_export.go:init()` — GDPR cache cleanup (5min ticker). Tightly coupled to package-level state. Leave as-is.
- `auth/local.go:init()` — Login state cleanup. Tightly coupled. Leave as-is.
- `mw/ratelimit.go` goroutines — Rate limiter cleanup. Leave as-is.
- These can be migrated to the scheduler in a follow-up if desired, but require more refactoring.

**Not a concern:**
- `backend/handlers/scheduling/time-blockers.go` already uses `robfig/cron/v3` — no new dependency, no version conflict.
- All 9 cleanup functions have signature `func(context.Context) error` — perfectly uniform for the `Job.Handler` type.
- The `scheduling` package is already imported in `main.go` — no new import needed for that.

**Key invariant:** All cleanup functions are idempotent (documented in code). Running them on cron instead of on-demand has zero correctness impact — they produce the same result regardless of how often they run.
