fix: address final-review findings on the latest-chapter poller
Wires the five poll env vars into docker-compose (the documented kill switch was inert), skips fetching unknown sites and non-https URLs before spending a request, and makes runOnce's summary log fire on empty and cancelled ticks. Records the accepted non-atomic Get+Upsert window and the one-interval startup delay in the docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -45,6 +45,10 @@ WEB_PASSWORD=
|
||||
# LATEST_CHAPTER_POLL_BATCH=14 # series per wake
|
||||
# LATEST_CHAPTER_POLL_STAGGER=20s # delay between fetches in a batch
|
||||
#
|
||||
# Uses a ticker, not an immediate first run: the first poll happens one
|
||||
# INTERVAL after startup, not at startup. A container restarting more often
|
||||
# than INTERVAL never polls.
|
||||
#
|
||||
# BATCH x (COOLDOWN / INTERVAL) series hold the cooldown cadence — 84 with these
|
||||
# defaults. Beyond that the cadence stretches uniformly rather than breaking;
|
||||
# raise BATCH or lower INTERVAL. Keep BATCH x STAGGER under INTERVAL.
|
||||
|
||||
@@ -51,6 +51,11 @@ Bromite userscript (isolated world, per-site adapters, localStorage cache)
|
||||
Fetches use `bogdanfinn/tls-client` with a Chrome profile as defence in depth
|
||||
against fingerprint-based blocking; any failure logs and skips. See
|
||||
`docs/superpowers/specs/2026-07-26-server-latest-chapter-polling-design.md`.
|
||||
The poller's `Store.Get` + `Store.Upsert` is not wrapped in a transaction, so
|
||||
a userscript `PUT` that commits between the two can be overwritten by the
|
||||
poller's stale re-read — reverting that read progress and, since the stored
|
||||
value now differs, moving `updated_at` and reordering the list. This is a
|
||||
known, accepted limitation for a single-user deployment, not a bug to fix.
|
||||
- **`updated_at` drives list order, so it moves only on real reading progress:** the server applies its timestamp when the row is new or `last_chapter_num` changes, and otherwise keeps the stored value — favouriting a series or recording a newly published chapter must not reorder the list. `PUT` therefore returns the row **as stored**, and clients must adopt that response rather than their own payload. See `plans/2026-07-25-bookmark-list-favorites-design.md` §4.
|
||||
- **Config via env:** `API_TOKEN`, `ALLOWED_ORIGINS` (comma list), `DB_PATH`
|
||||
(default `/data/bookmarks.db`), `PORT` (default `8080`), `WEB_PASSWORD`
|
||||
|
||||
+36
-4
@@ -3,6 +3,7 @@ package main
|
||||
import (
|
||||
"context"
|
||||
"log"
|
||||
"net/url"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -66,9 +67,6 @@ func (p *latestPoller) runOnce(ctx context.Context) {
|
||||
log.Printf("latest poll: due query: %v", err)
|
||||
return
|
||||
}
|
||||
if len(due) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
checked := 0
|
||||
for i, b := range due {
|
||||
@@ -79,13 +77,17 @@ func (p *latestPoller) runOnce(ctx context.Context) {
|
||||
// from one server IP is the traffic shape most likely to move that IP's
|
||||
// bot score. This is the server-side analogue of the userscript's "one
|
||||
// series per navigation ... indistinguishable from browsing" (L455-456).
|
||||
stopped := false
|
||||
if i > 0 && p.stagger > 0 {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
stopped = true
|
||||
case <-time.After(p.stagger):
|
||||
}
|
||||
}
|
||||
if stopped {
|
||||
break
|
||||
}
|
||||
p.checkOne(ctx, b)
|
||||
checked++
|
||||
}
|
||||
@@ -114,6 +116,18 @@ func (p *latestPoller) checkOne(ctx context.Context, b Bookmark) {
|
||||
return
|
||||
}
|
||||
|
||||
// series_url is client-supplied (PUT /bookmarks/{key} accepts any string),
|
||||
// so this is not just an optimisation against burning a request on an
|
||||
// unknown site: without it, the server would issue a GET from its own
|
||||
// network position to whatever URL a token-holder writes, including
|
||||
// link-local/internal addresses or non-https schemes. The cooldown above
|
||||
// is already consumed, so a row that never passes this check is retried at
|
||||
// cooldown pace rather than hot-looping.
|
||||
if !fetchableSeriesURL(b.Site, b.SeriesURL) {
|
||||
log.Printf("latest poll %q: not fetchable: site=%q url=%q", b.Key, b.Site, b.SeriesURL)
|
||||
return
|
||||
}
|
||||
|
||||
body, status, err := p.fetch.Get(ctx, b.SeriesURL)
|
||||
if err != nil {
|
||||
log.Printf("latest poll %q: fetch %s: %v", b.Key, b.SeriesURL, err)
|
||||
@@ -160,3 +174,21 @@ func (p *latestPoller) checkOne(ctx context.Context, b Bookmark) {
|
||||
}
|
||||
log.Printf("latest poll %q: latest is now %s", b.Key, latest.Label)
|
||||
}
|
||||
|
||||
// fetchableSeriesURL reports whether site is a site latestChapterFrom knows how
|
||||
// to parse and seriesURL is safe to hand to the fetcher: an https URL with a
|
||||
// non-empty host. series_url comes from client-supplied PUT bodies, so this is
|
||||
// a defence against the poller being used to probe arbitrary hosts from the
|
||||
// server's own network position, not just a check against wasted requests.
|
||||
func fetchableSeriesURL(site, seriesURL string) bool {
|
||||
switch site {
|
||||
case "asura", "demonic":
|
||||
default:
|
||||
return false
|
||||
}
|
||||
u, err := url.Parse(seriesURL)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return u.Scheme == "https" && u.Host != ""
|
||||
}
|
||||
|
||||
@@ -33,9 +33,12 @@ var demonicChapterRe = regexp.MustCompile(`chaptered\.php\?manga=\d+&(?:amp;)?ch
|
||||
// The userscript's asura rule additionally requires the anchor text to match
|
||||
// /Chapter\s+[\d.]+/i. That check exists only to skip the "First Chapter"
|
||||
// shortcut, which points at chapter/1 and therefore can never win a maximum, so
|
||||
// it is redundant here. Scoping the pattern to this series' own slug replaces it
|
||||
// with a stronger guarantee: a chapter link belonging to some other series
|
||||
// cannot contribute even if the page starts carrying them.
|
||||
// it is redundant here. For asura, scoping the pattern to this series' own slug
|
||||
// replaces it with a stronger guarantee: a chapter link belonging to some other
|
||||
// series cannot contribute even if the page starts carrying them. demonic has no
|
||||
// such guarantee — demonicChapterRe matches any chaptered.php?manga=<id> anchor
|
||||
// with no per-series scoping, because the stored series_id for demonic is a
|
||||
// slug, not the numeric id the URL carries, so it cannot easily be scoped.
|
||||
func latestChapterFrom(site, seriesURL, body string) (latestChapter, bool) {
|
||||
var re *regexp.Regexp
|
||||
switch site {
|
||||
|
||||
@@ -259,6 +259,48 @@ func TestRunOnceCorrectsDownward(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// series_url is client-supplied via PUT /bookmarks/{key}, so checkOne must
|
||||
// reject anything that is not a known site with an https URL before spending a
|
||||
// request on it — the cooldown still gets consumed either way.
|
||||
func TestCheckOneValidatesSeriesURLBeforeFetching(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
site string
|
||||
seriesURL string
|
||||
wantCalls int
|
||||
}{
|
||||
{"unknown site", "mangadex", "https://mangadex.org/title/x", 0},
|
||||
{"http scheme", "asura", "http://asurascans.com/comics/x", 0},
|
||||
{"unparseable url", "asura", "http://[::1", 0},
|
||||
{"valid https asura", "asura", "https://asurascans.com/comics/x", 1},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
key := tt.site + ":x"
|
||||
if _, err := s.Upsert(Bookmark{
|
||||
Key: key, Site: tt.site, SeriesID: "x", SeriesURL: tt.seriesURL,
|
||||
UpdatedAt: 1000,
|
||||
}); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
now := time.UnixMilli(4_000_000)
|
||||
f := &fakeFetcher{body: asuraSeriesFixture, status: 200}
|
||||
newTestPoller(t, s, f, now).checkOne(context.Background(), Bookmark{
|
||||
Key: key, Site: tt.site, SeriesURL: tt.seriesURL,
|
||||
})
|
||||
|
||||
if got := f.callCount(); got != tt.wantCalls {
|
||||
t.Fatalf("fetch calls = %d, want %d", got, tt.wantCalls)
|
||||
}
|
||||
if got := readLatestCheckedAt(t, s, key); got != now.UnixMilli() {
|
||||
t.Fatalf("latest_checked_at = %d, want %d (cooldown must be consumed regardless)", got, now.UnixMilli())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A cancelled context must abandon the batch rather than run it to completion.
|
||||
func TestRunOnceStopsOnCancelledContext(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
|
||||
@@ -20,6 +20,13 @@ services:
|
||||
PORT: "8080"
|
||||
# Gates the browser UI. Unset means the web routes are not served at all.
|
||||
WEB_PASSWORD: ${WEB_PASSWORD:-}
|
||||
# Latest-chapter poller. LATEST_CHAPTER_POLL_ENABLED=0 in .env is the kill
|
||||
# switch; it only takes effect because these are listed here.
|
||||
LATEST_CHAPTER_POLL_ENABLED: ${LATEST_CHAPTER_POLL_ENABLED:-1}
|
||||
LATEST_CHAPTER_POLL_COOLDOWN: ${LATEST_CHAPTER_POLL_COOLDOWN:-1h}
|
||||
LATEST_CHAPTER_POLL_INTERVAL: ${LATEST_CHAPTER_POLL_INTERVAL:-10m}
|
||||
LATEST_CHAPTER_POLL_BATCH: ${LATEST_CHAPTER_POLL_BATCH:-14}
|
||||
LATEST_CHAPTER_POLL_STAGGER: ${LATEST_CHAPTER_POLL_STAGGER:-20s}
|
||||
volumes:
|
||||
- bookmarks-data:/data
|
||||
# Bound to loopback only: the proxy (or curl during smoke test) reaches it,
|
||||
|
||||
Reference in New Issue
Block a user