Fix background feed refresh stopping after the first tick

[cubny]

Problem

Feeds never auto-update in production; the only way to refresh is pressing "update all", one feed at a time. Reported as the app being stuck for weeks.

Root cause

Scheduler.Queue is an unbuffered channel with exactly one send in the entire repo — the startup job in initScheduler (internal/app.go). The ticker arm of the worker loop did:

case <-ticker.C:
    for job := range s.Queue {   // ranges an open channel with no senders
        job.Execute()
    }

At boot the case job := <-s.Queue arm receives ItemsJob and runs it, so feeds refresh once at deploy time. One hour later the ticker fires, the goroutine enters range s.Queue, and blocks permanently. It never returns to the select, so no further tick is ever observed and no feed is ever refreshed again.

fly.toml is not implicated — min_machines_running = 1 keeps the machine up.

Two bugs fall out of the same blocked goroutine:

1. Feeds never auto-refresh. ("update all" works because the HTTP handler is a separate code path.)
2. Scheduler.Stop() deadlocks after the first tick: it does close(s.quit) then <-s.done, but the worker is parked in range s.Queue and never observes quit. Every fly.io deploy hung on shutdown until the machine was SIGKILLed.

Change

• scheduler.go: replace the channel dispatch with a job list fixed at construction — NewScheduler(interval, jobs...). Jobs run once at Start, then on every tick, and the loop always returns to select so quit is observed. ScheduleOnce (zero callers) and Queue are removed; jobs are immutable after construction, so no mutex is needed.
• app.go: initScheduler builds the job first and passes it to the constructor. The 1h interval is unchanged and stays hardcoded.
• itemsjob.go: ListFeeds failing for one user did a bare return with no log line, abandoning every subsequent user. Now logs and continues, matching the other error paths. This never bit us while the ticker was dead, but would once the loop runs.

Verification

• New scheduler_test.go — six tests, internal/infra/job now at 100% statement coverage. Two are direct regression guards: one asserts the job keeps firing across ticks (the old code records 1 execution across 5 ticks), the other asserts Stop() returns after a tick has fired.
• itemsjob_test.go's FailListFeeds case extended to two users, asserting the second user's feeds are still fetched and upserted.
• Full unit suite passes under -race; all 26 Playwright UI tests pass.
• Ran the app against the test server with the interval temporarily at 5s and watched it refresh at 21:35:02, :07, :12, :17, :22, then reverted to 1h.

Note: make lint is broken in this checkout for unrelated reasons — the installer fails a SHA256 check on the golangci-lint v2.12.2 tarball, and the locally installed binary (built with go1.25) refuses to run against this module's go1.26 target. go vet is clean; CI runs the real lint.

[Copilot]

Pull request overview

This PR fixes production background feed refresh by redesigning the job scheduler so it continues executing jobs on every tick and can shut down cleanly after ticks have fired.

Changes:

• Replaced the scheduler’s channel-based dispatch with a fixed job list provided at construction time, ensuring the worker loop always returns to select and observes quit.
• Added focused regression tests to ensure jobs run at startup, keep firing across ticks, and Stop() reliably returns.
• Updated ItemsJob to log and continue when ListFeeds fails for a user, preventing subsequent users from being skipped.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/infra/job/scheduler.go	Refactors scheduler to execute a fixed set of jobs at start and on each tick; removes queue-based dispatch.
internal/infra/job/scheduler_test.go	Adds regression and behavioral tests for tick execution and shutdown behavior.
internal/infra/job/itemsjob.go	Ensures per-user ListFeeds errors don’t abort processing of remaining users.
internal/infra/job/itemsjob_test.go	Extends coverage to verify one user’s failure doesn’t starve later users.
internal/app.go	Updates scheduler initialization to pass the constructed job into NewScheduler.

[cubny]

Good catch — fixed in
6662e32.

You're right that the lock-free read of jobs depends on the list being immutable after construction, and storing the variadic slice directly only made that true by coincidence: no current caller spreads a slice, so nothing can alias it today. That's a fragile thing to rest the no-mutex design on, so NewScheduler now does slices.Clone(jobs).

Added TestScheduler_DoesNotAliasCallerSlice as a guard. It constructs with jobs..., has the caller overwrite jobs[0] immediately after, and asserts the scheduler still runs the originally-registered job. Verified it's a real guard: red without the clone (the scheduler ran the swapped-in job), green with it.