diff --git a/.env.example b/.env.example index 123ac7c..6b9c726 100644 --- a/.env.example +++ b/.env.example @@ -100,7 +100,13 @@ DISCORD_REDIRECT_URI= # every kagane poll. Left unset here on purpose — a wrong default would poll a # stranger's address, and "no browser" is a safe, self-announcing state. # BROWSER_WS_URL=ws://100.x.y.z:9222 - +# +# Discord webhook for owner notices (outbound alerting when a poll Lane +# stalls). Unset means the whole path is off — a local stack needs no webhook, +# exactly as the browser URL behaves. The address is a secret in the class of +# TOKEN_KEY: never commit it, never paste it anywhere public. +# DISCORD_WEBHOOK_URL=https://discord.com/api/webhooks/... +# # Zone the backend stamps its log lines in. Cosmetic only. Nothing else in # the service has a zone: bookmark timestamps are unix ms, and the two real # time columns are timestamptz. Defaults to Asia/Jakarta; set to UTC for the diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 7501da1..247ee7e 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -159,6 +159,15 @@ failures. The flag decays and they probe again. - Browser Lanes wake Chrome only when 5+ Series are due or one has waited 15m, and cover work runs in the background so a slow CDN can't eat a Lane's gap. +### Owner notices — `internal/notify`, `latest.Fault`, `latest.Notifier`, `latest.FaultsFrom` + +The poller's outbound owner-notice path (issue #171): one condition today +(the stall), judged from the durable pass log alone so the poller and any +future reader of the same judgement cannot disagree. The webhook address is a +secret in the class of TOKEN_KEY — never logged, never rendered, never +carried in an error. The `owner_notices` suppression table (one row per +condition + site) is the only state; every threshold is derived, not stored. + ### Covers — `Store.OnSeriesCreated`, `latest.Acquirer`, `latest.CoverBytesFetcher`, `Store.SetSeriesCover` Acquired once when the first Bookmark of a Series is created, then served from diff --git a/backend/internal/latest/faults.go b/backend/internal/latest/faults.go new file mode 100644 index 0000000..88c810f --- /dev/null +++ b/backend/internal/latest/faults.go @@ -0,0 +1,101 @@ +package latest + +import ( + "context" + "fmt" + "time" + + "bookmarkmanager/backend/internal/store" +) + +// ConditionStall is the owner-notice machine word for a Lane that owed Polls, +// made none, and has nothing to say for it. The word is the message's footer +// and its suppression key; it is wire-stable. #172 declares the other three +// words (no-browser-route, sidecar-down, adapter-broken); this ticket +// declares only the stall. +const ConditionStall = "stall" + +// OwnerWindow is the class-level staleness boundary every owner-notice +// condition measures against — the same twelve hours the Lanes page's +// "not checked in 12h" filter uses (internal/web/admin.go). Declared here +// once so #172's three conditions and the admin filters share one figure. +const OwnerWindow = 12 * time.Hour + +// Fault is one condition the owner is told about, judged from durable rows +// alone. Site is "" for a fault that is not one Site's. +type Fault struct { + Condition string // one of the Condition* words + Site string + Since int64 // unix ms the episode began; the message's age +} + +// FaultInput is everything the judgement reads. A struct so #172's three +// conditions can add inputs without changing either caller. +type FaultInput struct { + Passes []store.LanePass +} + +// FaultsFrom judges the owner-notice conditions from durable rows alone, so +// the poller and the landing page cannot disagree about what a fault is. +// +// A Lane stalls when its latest pass shows due > 0, none checked, no skip +// value and no refusal — exactly the row the mid-loop browser loss writes +// (see the comment at the outcomeUnreachable return in runLanePass), so a +// Lane that owed Polls, made none, and has nothing to say for it is a fault. +// The no-refusal clause is AC6's amendment: the twice-refused break happens +// inside the pass loop, not on an early return, so a newly-gated Site would +// otherwise send two messages. A pause is excluded by Skip == "" (SkipPaused): +// the owner's own act is not reported back (AC10). The episode began at the +// pass that produced the row; the stall test itself has no age clause — the +// class-level twelve hours belongs to #172's conditions. +func FaultsFrom(in FaultInput, now time.Time) []Fault { + var faults []Fault + for _, p := range in.Passes { + if p.Due > 0 && p.Checked == 0 && p.Skip == "" && p.Refused == 0 { + faults = append(faults, Fault{Condition: ConditionStall, Site: p.Site, Since: p.RanAt}) + } + } + return faults +} + +// Notifier delivers one owner notice. The poller neither retries nor queues: +// an error is logged and the suppression row left unwritten, so the next pass +// tries again while the condition holds. +type Notifier interface { + Notify(ctx context.Context, f Fault, sentence, href string) error +} + +// ownerNoticeConditions is every condition this package judges, for the +// clear loop in recordPass: a condition absent from a pass's fault list +// forgets its episode, so the next occurrence sends again. #172 extends the +// list when it adds its conditions. +var ownerNoticeConditions = []string{ConditionStall} + +// noticeFor renders one fault's message: the description sentence — condition, +// age, repair — and the deep link the embed's title points at. Each condition +// provides its own wording; the stall is the only one today (issue #171). +func noticeFor(f Fault, row store.LanePass, now time.Time) (sentence, href string) { + switch f.Condition { + case ConditionStall: + return fmt.Sprintf( + "%s owed %d Polls and made none — %s; check the Lane's browser sidecar and the Site's challenge state", + f.Site, row.Due, humanAge(now.Sub(time.UnixMilli(f.Since)))), "/admin/lanes" + } + return "", "" +} + +// humanAge renders a duration the way an owner reads it in a message: minutes +// under an hour, then hours, then days; a stall that was just born reads +// "just now". +func humanAge(d time.Duration) string { + switch { + case d < time.Minute: + return "just now" + case d < time.Hour: + return fmt.Sprintf("%dm", int(d.Minutes())) + case d < 24*time.Hour: + return fmt.Sprintf("%dh", int(d.Hours())) + default: + return fmt.Sprintf("%dd", int(d.Hours()/24)) + } +} diff --git a/backend/internal/latest/poller.go b/backend/internal/latest/poller.go index fff24dc..f43d4b3 100644 --- a/backend/internal/latest/poller.go +++ b/backend/internal/latest/poller.go @@ -46,7 +46,10 @@ type Poller struct { // CoverBytesFetch is optional; it handles plain-TLS sources through the // same failure-isolated prefetch path. CoverBytesFetch CoverBytesFetcher - Now func() time.Time // injected so tests can freeze it + // Notify delivers owner notices. Nil disables the whole path (issue #171): + // the poller is not the place a missing webhook becomes an error. + Notify Notifier + Now func() time.Time // injected so tests can freeze it // eligibleCount reports how many of a Site's Series are eligible for // polling, defaulting to Store.EligibleSeriesCount. Injected so tests can // fail the count alone: the eligible query shares the due query's tables, @@ -357,7 +360,7 @@ func (p *Poller) runLanePass(ctx context.Context, name string, paced bool) time. // carries the previous pass's forward inside recordPass. fig := passFigures{} rec := passRecord{site: name, ranAt: now.UnixMilli()} - defer func() { p.recordPass(rec, fig) }() + defer func() { p.recordPass(ctx, rec, fig) }() // One Lane row read at the top of a pass, serving two gates (issue #139). // Both stamps outlive our process, so the gates read the durable row @@ -527,8 +530,9 @@ type passFigures struct { // pass's due, gap, clamped and checked forward rather than stating zeroes it // did not measure; the skip column says why it declined, so the zeroes that // remain (due-query, no-fetcher) read as explanations rather than -// measurements. -func (p *Poller) recordPass(rec passRecord, fig passFigures) { +// measurements. Once the row is durable, the owner-notice judgement runs +// beside it (issue #171). +func (p *Poller) recordPass(ctx context.Context, rec passRecord, fig passFigures) { row := store.LanePass{ Site: rec.site, RanAt: rec.ranAt, @@ -557,7 +561,65 @@ func (p *Poller) recordPass(rec passRecord, fig passFigures) { } if err := p.Store.RecordLanePass(row, rec.ranAt-lanePassRetention.Milliseconds()); err != nil { log.Printf("latest poll %s: record lane pass: %v", rec.site, err) + return } + p.ownerNotices(ctx, row) +} + +// ownerNotices judges the owner-notice conditions for the pass just recorded +// and fires (issue #171). It sits in recordPass because that deferred call is +// the one place every return path passes through: two of the four conditions +// occur on early returns and the success path can never see them. Per fault: +// NoticeSent → send → MarkNoticeSent, so a fault lasting a month sends one +// message, not one per pass; a condition absent from this pass's fault list +// forgets its episode, so the next occurrence sends again. Everything here is +// best-effort: a failed send, a failed store read and a failed notice write +// are all logged and never change the pass's outcome counts or its return +// value. The clear runs even when Notify is nil, so a deployment that turns +// the webhook off does not leave stale rows that suppress the first real +// notice after it is turned back on. +func (p *Poller) ownerNotices(ctx context.Context, row store.LanePass) { + faults := FaultsFrom(FaultInput{Passes: []store.LanePass{row}}, p.Now()) + for _, f := range faults { + if p.Notify == nil { + continue + } + sent, err := p.Store.NoticeSent(f.Condition, f.Site) + if err != nil { + log.Printf("latest poll %s: notice sent: %v", row.Site, err) + continue + } + if sent { + continue + } + sentence, href := noticeFor(f, row, p.Now()) + if err := p.Notify.Notify(ctx, f, sentence, href); err != nil { + // The stamp stays unset: no queue, no backoff — the condition is + // durable, so the next pass tries again while it holds. + log.Printf("latest poll %s: owner notice %s: %v", row.Site, f.Condition, err) + continue + } + if err := p.Store.MarkNoticeSent(f.Condition, f.Site, p.Now().UnixMilli()); err != nil { + log.Printf("latest poll %s: mark notice sent: %v", row.Site, err) + } + } + for _, cond := range ownerNoticeConditions { + if !hasFault(faults, cond, row.Site) { + if err := p.Store.ClearNotice(cond, row.Site); err != nil { + log.Printf("latest poll %s: clear owner notice: %v", row.Site, err) + } + } + } +} + +// hasFault reports whether faults hold the given condition for the site. +func hasFault(faults []Fault, condition, site string) bool { + for _, f := range faults { + if f.Condition == condition && f.Site == site { + return true + } + } + return false } // countEligible routes the eligible count through the test seam when one is diff --git a/backend/internal/latest/poller_test.go b/backend/internal/latest/poller_test.go index d87b534..f7aff16 100644 --- a/backend/internal/latest/poller_test.go +++ b/backend/internal/latest/poller_test.go @@ -159,6 +159,35 @@ func (f *fakeBytesCoverFetcher) callCount() int { return len(f.calls) } +// fakeNotifier records the owner notices the poller sends, standing in for +// the Discord webhook the way fakeFetcher stands in for the network. An err +// set after construction makes every subsequent send fail, for the retry +// test. +type fakeNotifier struct { + mu sync.Mutex + calls []Fault + err error +} + +func (f *fakeNotifier) Notify(_ context.Context, fault Fault, _, _ string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.calls = append(f.calls, fault) + return f.err +} + +func (f *fakeNotifier) callCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.calls) +} + +func (f *fakeNotifier) fault(i int) Fault { + f.mu.Lock() + defer f.mu.Unlock() + return f.calls[i] +} + // newTestPoller wires a poller with a frozen clock. Rest and gap come from the // Site registry, so tests seed checked_at relative to the one-hour rest; the // round entry point (runOnce) runs every Lane back to back with no real @@ -3092,3 +3121,214 @@ func TestRunOnceSiteCompletedWritesDespiteUnchangedChapter(t *testing.T) { t.Fatalf("site_completed_at after unchanged-chapter completed poll = %d, want %d", got, at.UnixMilli()) } } + +// The stall judgement (issue #171) selects exactly the row the mid-loop +// browser loss writes — due > 0, none checked, no skip value, and no refusal. +// The no-refusal clause is AC6's amendment: the twice-refused break happens +// inside the pass loop, not on an early return, so a newly-gated Site would +// otherwise send two messages. A pause is excluded by Skip == "" (SkipPaused): +// the owner's own act is not reported back (AC10). +func TestFaultsFromStall(t *testing.T) { + now := time.UnixMilli(5_000_000) + tests := []struct { + name string + pass store.LanePass + want int + }{ + { + name: "stall-shaped pass is a stall", + pass: store.LanePass{Site: "comix", RanAt: now.UnixMilli(), Due: 2, Checked: 0, Skip: "", Refused: 0}, + want: 1, + }, + { + name: "nothing due is not a stall", + pass: store.LanePass{Site: "comix", RanAt: now.UnixMilli(), Due: 0, Checked: 0, Skip: "", Refused: 0}, + want: 0, + }, + { + name: "a pass that checked is not a stall", + pass: store.LanePass{Site: "comix", RanAt: now.UnixMilli(), Due: 1, Checked: 1, Skip: "", Refused: 0}, + want: 0, + }, + { + name: "paused is the owner's own act, not a stall", + pass: store.LanePass{Site: "comix", RanAt: now.UnixMilli(), Due: 1, Checked: 0, Skip: SkipPaused, Refused: 0}, + want: 0, + }, + { + name: "refusing has a skip value, not a stall", + pass: store.LanePass{Site: "comix", RanAt: now.UnixMilli(), Due: 1, Checked: 0, Skip: SkipRefusing, Refused: 0}, + want: 0, + }, + { + name: "stalled and refused fires nothing (AC6's collision)", + pass: store.LanePass{Site: "comix", RanAt: now.UnixMilli(), Due: 2, Checked: 0, Skip: "", Refused: 1}, + want: 0, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + faults := FaultsFrom(FaultInput{Passes: []store.LanePass{tt.pass}}, now) + if got := len(faults); got != tt.want { + t.Fatalf("FaultsFrom = %+v, want %d fault(s)", faults, tt.want) + } + for _, f := range faults { + if f.Condition != ConditionStall || f.Site != "comix" || f.Since != now.UnixMilli() { + t.Fatalf("fault = %+v, want stall on comix since the pass time", f) + } + } + }) + } +} + +// The stall fires exactly once while it holds, and again after it lifts and +// returns: the suppression row is the whole state, so the second stall-shaped +// pass is the same episode, a healthy pass in between clears the row, and the +// next stall is a new episode that sends again. +func TestOwnerNoticeStallFiresOncePerEpisode(t *testing.T) { + now := time.UnixMilli(5_000_000) + s, _ := newTestStore(t) + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + interrupted := fmt.Errorf("%w: %w", errBrowserInterrupted, errors.New("restart")) + notifier := &fakeNotifier{} + p := newTestPoller(t, s, &fakeFetcher{status: 200}, now) + p.Now = func() time.Time { return now } + p.BrowserFetch = &fakeFetcher{status: 200, err: interrupted} + p.Notify = notifier + + // First stall-shaped pass: the episode begins, the owner is told once. + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 1 { + t.Fatalf("notices after first stall = %d, want 1", got) + } + if f := notifier.fault(0); f.Condition != ConditionStall || f.Site != "comix" || f.Since != now.UnixMilli() { + t.Fatalf("fault = %+v, want stall on comix since the pass time", f) + } + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || !sent { + t.Fatalf("NoticeSent after first stall = %v, %v; want true, nil", sent, err) + } + + // A second stall-shaped pass — the series was stamped, so re-seed it — is + // the same episode: no second message. + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + now = now.Add(RefuseBackoff + time.Minute) + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 1 { + t.Fatalf("notices after second stall = %d, want 1 (one per episode)", got) + } + + // A healthy pass in between lifts the stall: the episode ends and the + // suppression row is cleared. + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + p.BrowserFetch = &fakeFetcher{body: comixSeriesFixture, status: 200} + now = now.Add(RefuseBackoff + time.Minute) + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 1 { + t.Fatalf("notices after healthy pass = %d, want 1 (a healthy pass sends nothing)", got) + } + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || sent { + t.Fatalf("NoticeSent after healthy pass = %v, %v; want false, nil (row cleared)", sent, err) + } + + // The next stall is a new episode: the owner is told again. + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + p.BrowserFetch = &fakeFetcher{status: 200, err: interrupted} + now = now.Add(RefuseBackoff + time.Minute) + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 2 { + t.Fatalf("notices after the next stall = %d, want 2 (again after it lifts and returns)", got) + } +} + +// A paused Lane sends nothing: the pause is the owner's own act, and the +// skip value already keeps it out of the stall test (AC10). +func TestOwnerNoticePausedSendsNothing(t *testing.T) { + now := time.UnixMilli(5_000_000) + s, _ := newTestStore(t) + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + if err := s.PauseLane("comix", now.Add(30*time.Minute).UnixMilli()); err != nil { + t.Fatalf("PauseLane: %v", err) + } + notifier := &fakeNotifier{} + p := newTestPoller(t, s, &fakeFetcher{status: 200}, now) + p.BrowserFetch = &fakeFetcher{status: 200} + p.Notify = notifier + + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 0 { + t.Fatalf("notices while paused = %d, want 0", got) + } + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || sent { + t.Fatalf("NoticeSent while paused = %v, %v; want false, nil", sent, err) + } +} + +// A failed POST is logged and the notified stamp left unset, so the next pass +// retries while the condition holds. No queue, no backoff — the condition is +// durable, and the suppression row is the only state. +func TestOwnerNoticeFailedSendRetriesNextPass(t *testing.T) { + now := time.UnixMilli(5_000_000) + s, _ := newTestStore(t) + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + interrupted := fmt.Errorf("%w: %w", errBrowserInterrupted, errors.New("restart")) + notifier := &fakeNotifier{err: errors.New("webhook down")} + p := newTestPoller(t, s, &fakeFetcher{status: 200}, now) + p.Now = func() time.Time { return now } + p.BrowserFetch = &fakeFetcher{status: 200, err: interrupted} + p.Notify = notifier + + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 1 { + t.Fatalf("send attempts = %d, want 1", got) + } + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || sent { + t.Fatalf("NoticeSent after failed send = %v, %v; want false, nil (stamp left unset)", sent, err) + } + + // The webhook recovers: the next stall-shaped pass delivers the one + // message the episode was owed. + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + notifier.err = nil + now = now.Add(RefuseBackoff + time.Minute) + p.runLanePass(context.Background(), "comix", false) + if got := notifier.callCount(); got != 2 { + t.Fatalf("send attempts after recovery = %d, want 2 (retried next pass)", got) + } + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || !sent { + t.Fatalf("NoticeSent after recovery = %v, %v; want true, nil", sent, err) + } +} + +// Nil notifier means the whole path is off: a stall-shaped pass panics +// nothing and stamps no row, yet the clear still runs, so a deployment that +// turns the webhook off does not leave stale rows that suppress the first +// real notice after it is turned back on. +func TestOwnerNoticeNilNotifierStillClears(t *testing.T) { + now := time.UnixMilli(5_000_000) + s, _ := newTestStore(t) + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + interrupted := fmt.Errorf("%w: %w", errBrowserInterrupted, errors.New("restart")) + + // A stall-shaped pass with no notifier configured: nothing panics and no + // row is stamped — a missing webhook is a silent, complete off switch. + p := newTestPoller(t, s, &fakeFetcher{status: 200}, now) + p.Now = func() time.Time { return now } + p.BrowserFetch = &fakeFetcher{status: 200, err: interrupted} + p.runLanePass(context.Background(), "comix", false) + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || sent { + t.Fatalf("NoticeSent with nil notifier = %v, %v; want false, nil", sent, err) + } + + // A stale row from before the webhook was turned off must not survive a + // healthy pass: the clear runs even though no notifier is configured. + if err := s.MarkNoticeSent(ConditionStall, "comix", now.UnixMilli()); err != nil { + t.Fatalf("seed notice row: %v", err) + } + seedForCheck(t, s, "comix:c", "https://comix.to/title/c", 0) + now = now.Add(RefuseBackoff + time.Minute) + p.BrowserFetch = &fakeFetcher{body: comixSeriesFixture, status: 200} + p.runLanePass(context.Background(), "comix", false) + if sent, err := s.NoticeSent(ConditionStall, "comix"); err != nil || sent { + t.Fatalf("NoticeSent after healthy pass with nil notifier = %v, %v; want false, nil (cleared)", sent, err) + } +} diff --git a/backend/internal/notify/notify.go b/backend/internal/notify/notify.go new file mode 100644 index 0000000..fedabc8 --- /dev/null +++ b/backend/internal/notify/notify.go @@ -0,0 +1,122 @@ +// Package notify posts owner-notice embeds to a Discord webhook. It is +// deliberately small and stdlib-only: one POST of a JSON body needs no +// Discord library, and the path imports nothing of this repo's session or +// OAuth packages — that independence is why a webhook was chosen over a bot +// (issue #171, AC9). +package notify + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "strings" + "time" + + "bookmarkmanager/backend/internal/latest" +) + +// dangerColor is the dark design branch's --danger, #cf5c4d = 13589581. The +// integer is unreadable, so a future edit will otherwise "fix" it — do not: +// --ember means new chapter only, and a fault wearing ember would tell the +// owner a stall is a release. This is the one colour a fault wears. +const dangerColor = 13589581 + +// Client posts owner notices to one Discord webhook. The webhook address is a +// secret in the class of TOKEN_KEY: it is never logged, never rendered, and +// never carried in a returned error, which the poller logs. +type Client struct { + webhookURL string + baseURL string // the deployment's public origin; embed URLs resolve against it + http *http.Client +} + +// New returns a Client posting to webhookURL. baseURL is the deployment's +// public origin (Config.PublicBaseURL); the embed's deep-linked title is +// built from it. +func New(webhookURL, baseURL string) *Client { + return &Client{ + webhookURL: webhookURL, + baseURL: strings.TrimSuffix(baseURL, "/"), + http: &http.Client{Timeout: 10 * time.Second}, + } +} + +// Notify posts one owner notice as a Discord embed: the danger colour, the +// subject as a deep-linked title, the sentence as the description, the pass +// time as the timestamp and the machine word in the footer. The send is +// wrapped in a deadline so a hanging Discord cannot hold a poller Lane. On +// failure the error carries no part of the webhook address (the poller logs +// it), and the caller leaves the suppression row unwritten so the next pass +// retries while the condition holds. +func (c *Client) Notify(ctx context.Context, f latest.Fault, sentence, href string) error { + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + body, err := json.Marshal(c.payload(f, sentence, href)) + if err != nil { + return fmt.Errorf("owner notice: marshal: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.webhookURL, strings.NewReader(string(body))) + if err != nil { + return errors.New("owner notice: build request") + } + req.Header.Set("Content-Type", "application/json") + resp, err := c.http.Do(req) + if err != nil { + // The transport error embeds the webhook address; the address is a + // secret in the class of TOKEN_KEY and the poller logs this error. + return errors.New("owner notice: send failed") + } + defer resp.Body.Close() + // Cap the read: a Discord error page is enough, and draining the body + // lets the connection be reused. + io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return fmt.Errorf("owner notice: webhook status %d", resp.StatusCode) + } + return nil +} + +// payload is the webhook body: one embed and nothing else. No fields, no +// thumbnail, no author block — the wire shape is what Discord reads. +func (c *Client) payload(f latest.Fault, sentence, href string) webhookPayload { + return webhookPayload{Embeds: []embed{{ + Color: dangerColor, + Title: subject(f), + URL: c.baseURL + href, + Description: sentence, + Timestamp: time.UnixMilli(f.Since).UTC().Format(time.RFC3339), + Footer: embedFooter{Text: f.Condition}, + }}} +} + +// subject renders the embed's title: the Site the fault is about, with the +// machine word so the title needs no per-condition wording here — #172's +// conditions pass different sentences, not a different builder. For a fault +// that is not one Site's, the machine word stands alone. +func subject(f latest.Fault) string { + if f.Site == "" { + return f.Condition + } + return f.Site + ": " + f.Condition +} + +type webhookPayload struct { + Embeds []embed `json:"embeds"` +} + +type embed struct { + Color int `json:"color"` + Title string `json:"title"` + URL string `json:"url"` + Description string `json:"description"` + Timestamp string `json:"timestamp"` + Footer embedFooter `json:"footer"` +} + +type embedFooter struct { + Text string `json:"text"` +} diff --git a/backend/internal/notify/notify_test.go b/backend/internal/notify/notify_test.go new file mode 100644 index 0000000..0658636 --- /dev/null +++ b/backend/internal/notify/notify_test.go @@ -0,0 +1,100 @@ +package notify_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "bookmarkmanager/backend/internal/latest" + "bookmarkmanager/backend/internal/notify" +) + +// TestNotifyEmbedShape pins the wire shape Discord actually reads: one embed +// in the danger colour with the subject as a deep-linked title built from the +// base URL, the sentence as the description, the pass time as an RFC3339 +// timestamp, the machine word in the footer, and no fields grid (nor +// thumbnail, nor author block) at all. The webhook is a local server, so no +// test can reach Discord. +func TestNotifyEmbedShape(t *testing.T) { + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if ct := r.Header.Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + defer r.Body.Close() + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decode request body: %v", err) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + passTime := time.UnixMilli(5_000_000).UTC() + c := notify.New(srv.URL, "https://bookmarks.test/") + err := c.Notify(context.Background(), + latest.Fault{Condition: latest.ConditionStall, Site: "comix", Since: passTime.UnixMilli()}, + "sentence", "/admin/lanes") + if err != nil { + t.Fatalf("Notify: %v", err) + } + + embeds, ok := body["embeds"].([]any) + if !ok || len(embeds) != 1 { + t.Fatalf("embeds = %#v, want exactly one embed", body["embeds"]) + } + embed, ok := embeds[0].(map[string]any) + if !ok { + t.Fatalf("embed = %#v, want an object", embeds[0]) + } + if color, ok := embed["color"].(float64); !ok || int(color) != 13589581 { + t.Fatalf("color = %#v, want 13589581 (--danger #cf5c4d)", embed["color"]) + } + if url := embed["url"]; url != "https://bookmarks.test/admin/lanes" { + t.Fatalf("url = %#v, want the title deep-linked off the base URL", url) + } + if desc := embed["description"]; desc != "sentence" { + t.Fatalf("description = %#v, want the sentence", desc) + } + footer, ok := embed["footer"].(map[string]any) + if !ok || footer["text"] != latest.ConditionStall { + t.Fatalf("footer = %#v, want the machine word in the footer", embed["footer"]) + } + ts, ok := embed["timestamp"].(string) + if !ok { + t.Fatalf("timestamp = %#v, want an RFC3339 string", embed["timestamp"]) + } + parsed, err := time.Parse(time.RFC3339, ts) + if err != nil || !parsed.Equal(passTime) { + t.Fatalf("timestamp = %q, want %s (the pass time, RFC3339)", ts, passTime.Format(time.RFC3339)) + } + for _, banned := range []string{"fields", "thumbnail", "author"} { + if _, ok := embed[banned]; ok { + t.Fatalf("embed has %q, want it absent (no field grid, no thumbnail, no author block)", banned) + } + } +} + +// A non-2xx answer is an error the caller logs, and the error never carries +// the webhook address — a secret in the class of TOKEN_KEY, and the poller +// logs every notify error. +func TestNotifyNon2xxIsErrorWithoutTheAddress(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + c := notify.New(srv.URL, "https://bookmarks.test") + err := c.Notify(context.Background(), + latest.Fault{Condition: latest.ConditionStall, Site: "comix", Since: 5_000_000}, + "sentence", "/admin/lanes") + if err == nil { + t.Fatal("Notify = nil, want an error for a 500") + } + if strings.Contains(err.Error(), srv.URL) { + t.Fatalf("error %q leaks the webhook address", err) + } +} diff --git a/backend/internal/store/migrations/0020_owner_notices.sql b/backend/internal/store/migrations/0020_owner_notices.sql new file mode 100644 index 0000000..4717ff4 --- /dev/null +++ b/backend/internal/store/migrations/0020_owner_notices.sql @@ -0,0 +1,12 @@ +-- Owner notices (issue #171): one row per episode, remembered only as "the +-- owner was told". The row's presence is the whole state — the poller checks +-- it before sending, writes it after a successful send, and clears it when +-- the condition no longer holds. site is '' for a fault that is not one +-- Site's; the composite primary key is what makes the row a lock against a +-- second message for the same episode. +CREATE TABLE owner_notices ( + condition text NOT NULL, + site text NOT NULL, + notified_at bigint NOT NULL, + PRIMARY KEY (condition, site) +); diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index 1f348f3..2f579a3 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -1329,6 +1329,46 @@ func (s *Store) ClearSeriesFailure(site, seriesID string) error { return nil } +// NoticeSent reports whether the owner has already been told about this +// episode (issue #171). The row's presence is the whole state, so a missing +// row reads false without error. +func (s *Store) NoticeSent(condition, site string) (bool, error) { + var at int64 + err := s.db.QueryRow( + `SELECT notified_at FROM owner_notices WHERE condition = $1 AND site = $2`, + condition, site).Scan(&at) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("notice sent %s/%s: %w", condition, site, err) + } + return true, nil +} + +// MarkNoticeSent records that the owner was told. Called only after a +// successful send. Re-marking the same episode just moves the stamp: the row +// is a lock against a second message, not a log. +func (s *Store) MarkNoticeSent(condition, site string, at int64) error { + if _, err := s.db.Exec(` + INSERT INTO owner_notices (condition, site, notified_at) VALUES ($1, $2, $3) + ON CONFLICT (condition, site) DO UPDATE SET notified_at = EXCLUDED.notified_at`, + condition, site, at); err != nil { + return fmt.Errorf("mark notice sent %s/%s: %w", condition, site, err) + } + return nil +} + +// ClearNotice forgets an episode, so the condition's next occurrence sends +// again. A row that is not there is not an error. +func (s *Store) ClearNotice(condition, site string) error { + if _, err := s.db.Exec( + `DELETE FROM owner_notices WHERE condition = $1 AND site = $2`, condition, site); err != nil { + return fmt.Errorf("clear notice %s/%s: %w", condition, site, err) + } + return nil +} + // DueForLatestCheck returns one Site's series whose server-side // latest-chapter check has aged past cutoffMs, ordered by how many bookmarks // reference them (descending) then least-recently-checked first. One Site per diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 286c212..b70093d 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -2700,3 +2700,41 @@ func failureRow(t *testing.T, s *Store, site, seriesID string) (word string, sin } return word, since, true } + +// Owner notices are one row per episode: the row's presence is the whole +// state. MarkNoticeSent stamps it, NoticeSent reads it, and ClearNotice +// forgets it — including a row that was never there, which is not an error. +func TestOwnerNoticeRows(t *testing.T) { + s := newTestStore(t) + + if sent, err := s.NoticeSent("stall", "comix"); err != nil || sent { + t.Fatalf("NoticeSent on a fresh table = %v, %v; want false, nil", sent, err) + } + + if err := s.MarkNoticeSent("stall", "comix", 4242); err != nil { + t.Fatalf("MarkNoticeSent: %v", err) + } + if sent, err := s.NoticeSent("stall", "comix"); err != nil || !sent { + t.Fatalf("NoticeSent after mark = %v, %v; want true, nil", sent, err) + } + + // Re-marking the same episode just moves the stamp: the row is a lock, + // not a log. A different site is its own episode — the key is + // (condition, site). + if err := s.MarkNoticeSent("stall", "comix", 4343); err != nil { + t.Fatalf("re-mark: %v", err) + } + if sent, err := s.NoticeSent("stall", "kagane"); err != nil || sent { + t.Fatalf("NoticeSent on another site = %v, %v; want false, nil", sent, err) + } + + if err := s.ClearNotice("stall", "comix"); err != nil { + t.Fatalf("ClearNotice: %v", err) + } + if sent, err := s.NoticeSent("stall", "comix"); err != nil || sent { + t.Fatalf("NoticeSent after clear = %v, %v; want false, nil", sent, err) + } + if err := s.ClearNotice("stall", "comix"); err != nil { + t.Fatalf("ClearNotice on a missing row: %v", err) + } +} diff --git a/backend/internal/web/admin.go b/backend/internal/web/admin.go index 4ef0dff..887962c 100644 --- a/backend/internal/web/admin.go +++ b/backend/internal/web/admin.go @@ -4,14 +4,16 @@ import ( "log" "net/http" "strconv" - "time" + "bookmarkmanager/backend/internal/latest" "bookmarkmanager/backend/internal/store" ) // ownerWindow is the staleness boundary the Series list's "not checked in -// 12h" filter compares against. Declared once; later admin tickets read it. -const ownerWindow = 12 * time.Hour +// 12h" filter compares against. It reads latest.OwnerWindow — the one place +// the class-level twelve hours lives, shared with the owner-notice +// conditions (issue #171). +const ownerWindow = latest.OwnerWindow // adminView is the shared shell data for an administrative page and the roster // fragment returned after a Reader action. diff --git a/backend/main.go b/backend/main.go index 629e1cf..7d265ae 100644 --- a/backend/main.go +++ b/backend/main.go @@ -14,6 +14,7 @@ import ( "bookmarkmanager/backend/internal/api" "bookmarkmanager/backend/internal/httpmw" "bookmarkmanager/backend/internal/latest" + "bookmarkmanager/backend/internal/notify" "bookmarkmanager/backend/internal/store" "bookmarkmanager/backend/internal/token" "bookmarkmanager/backend/internal/userscript" @@ -59,6 +60,12 @@ type Config struct { // the Lanes page reports the fact and derives reachability from the pass // log rather than asking the poller (issue #145). BrowserWSURL string + // DiscordWebhookURL is the webhook owner notices post to (issue #171). + // Unset means the whole path is off — a local stack needs no webhook, + // exactly as the browser URL behaves. The address is a secret in the + // class of TOKEN_KEY: never logged, and it must not reach any line that + // prints configuration. + DiscordWebhookURL string // LatestPoll configures the background latest-chapter fetcher. LatestPoll LatestPoll } @@ -114,6 +121,7 @@ func loadConfig() Config { UserscriptPath: envOr("USERSCRIPT_PATH", "/userscript/manga-bookmark.user.js"), NovelUserscriptPath: envOr("NOVEL_USERSCRIPT_PATH", "/userscript/novel-bookmark.user.js"), BrowserWSURL: os.Getenv("BROWSER_WS_URL"), + DiscordWebhookURL: os.Getenv("DISCORD_WEBHOOK_URL"), LatestPoll: loadLatestPoll(), } c.Discord = web.DiscordConfig{ @@ -288,10 +296,22 @@ func main() { } s.OnSeriesCreated = acq.Acquire } + // Owner notices (issue #171): a configured webhook makes the poller tell + // the owner about stalled Lanes. Unset means the whole path is off — a + // local stack needs no webhook, exactly as the browser URL behaves. Only + // the presence is logged; the address itself is a secret in the class of + // TOKEN_KEY. + var notifier latest.Notifier + if u := strings.TrimSpace(cfg.DiscordWebhookURL); u != "" { + notifier = notify.New(u, cfg.PublicBaseURL) + log.Println("owner notices: enabled") + } else { + log.Println("owner notices: disabled (DISCORD_WEBHOOK_URL unset)") + } // The poller's only connection to the web layer is the database now: it is // started for its own sake, and the Lanes page reads the pass rows it // records (issue #145). - startLatestPoller(pollCtx, s, cfg.LatestPoll, browser) + startLatestPoller(pollCtx, s, cfg.LatestPoll, browser, notifier) srv := &http.Server{ Addr: ":" + cfg.Port, @@ -324,7 +344,9 @@ func main() { // newLatestPoller wires the fetcher seams into the poller. Pace is registry // property, not config (issue #100), so there are no knobs to pass through. -func newLatestPoller(s *store.Store, cfg LatestPoll, fetch, browser latest.Fetcher) *latest.Poller { +// notifier is nil when no webhook is configured: a missing webhook is a +// silent off switch, not an error (issue #171). +func newLatestPoller(s *store.Store, cfg LatestPoll, fetch, browser latest.Fetcher, notifier latest.Notifier) *latest.Poller { var covers latest.BrowserCoverFetcher if f, ok := browser.(latest.BrowserCoverFetcher); ok { covers = f @@ -335,6 +357,7 @@ func newLatestPoller(s *store.Store, cfg LatestPoll, fetch, browser latest.Fetch BrowserFetch: browser, CoverFetch: covers, CoverBytesFetch: latest.NewCoverFetcher(), + Notify: notifier, Now: time.Now, } } @@ -345,7 +368,8 @@ func newLatestPoller(s *store.Store, cfg LatestPoll, fetch, browser latest.Fetch // tracking, which is exactly how it behaved before. It returns the running // Poller, or nil when there is none; the caller starts it for its own sake — // the Lanes page reads the pass log, so no return value is wired anywhere. -func startLatestPoller(ctx context.Context, s *store.Store, cfg LatestPoll, browser latest.Fetcher) *latest.Poller { +// notifier is nil when DISCORD_WEBHOOK_URL is unset (issue #171). +func startLatestPoller(ctx context.Context, s *store.Store, cfg LatestPoll, browser latest.Fetcher, notifier latest.Notifier) *latest.Poller { if !cfg.Enabled { log.Println("latest-chapter poller: disabled by config") return nil @@ -358,7 +382,7 @@ func startLatestPoller(ctx context.Context, s *store.Store, cfg LatestPoll, brow // Nil browser: sites behind a JavaScript challenge are simply not polled, // and their latest_chapter comes from the userscript alone — which is how // the service behaved before the sidecar existed. - p := newLatestPoller(s, cfg, f, browser) + p := newLatestPoller(s, cfg, f, browser, notifier) go p.Run(ctx) return p diff --git a/backend/main_test.go b/backend/main_test.go index f2a0146..e07917f 100644 --- a/backend/main_test.go +++ b/backend/main_test.go @@ -28,6 +28,20 @@ func TestLoadConfigReadsCoverDirectory(t *testing.T) { } } +func TestLoadConfigReadsDiscordWebhook(t *testing.T) { + // The address is read, never defaulted: unset stays empty (the whole + // path is off), set flows into the Config for the poller's notifier. + const url = "https://discord.com/api/webhooks/000000/secret" + t.Setenv("DISCORD_WEBHOOK_URL", url) + if got := loadConfig().DiscordWebhookURL; got != url { + t.Fatalf("DiscordWebhookURL = %q, want %q", got, url) + } + t.Setenv("DISCORD_WEBHOOK_URL", "") + if got := loadConfig().DiscordWebhookURL; got != "" { + t.Fatalf("DiscordWebhookURL = %q, want empty when unset", got) + } +} + func TestLoadLatestPollEnabledParsing(t *testing.T) { tests := []struct { raw string @@ -51,7 +65,8 @@ func TestLoadLatestPollEnabledParsing(t *testing.T) { // nothing here sizes a cooldown any more. func TestNewLatestPollerWiresFetchers(t *testing.T) { tls := &latest.TLSFetcher{} - p := newLatestPoller(nil, LatestPoll{Enabled: true}, tls, nil) + notifier := &stubNotifier{} + p := newLatestPoller(nil, LatestPoll{Enabled: true}, tls, nil, notifier) if p.Fetch != tls { t.Fatalf("Fetch not wired") } @@ -67,8 +82,15 @@ func TestNewLatestPollerWiresFetchers(t *testing.T) { if p.Now == nil { t.Fatalf("Now = nil, want the live clock") } + if p.Notify != notifier { + t.Fatalf("Notify = %v, want the configured notifier", p.Notify) + } } +// stubNotifier satisfies latest.Notifier so newLatestPoller's wiring can be +// asserted; it is never called. +type stubNotifier struct{ latest.Notifier } + func TestPutStatusValidation(t *testing.T) { cases := []struct { name string diff --git a/docker-compose.yml b/docker-compose.yml index de80e68..eb77921 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -75,7 +75,11 @@ services: # Host header is an IP address or "localhost" — confirmed 2026-08-03, # independent of chromedp's own dial logic. The same trap that used to # force a pinned Docker IP now forbids the tailnet name. - BROWSER_WS_URL: ${BROWSER_WS_URL:-} + # Owner-notice webhook (issue #171). Empty default, never a + # required-guard: unset means the whole path is off, so a local stack + # runs exactly as it does today. An env var not listed here never + # reaches the container. + DISCORD_WEBHOOK_URL: ${DISCORD_WEBHOOK_URL:-} depends_on: # The migration runner is the first thing the binary does, so a Postgres # that is still initialising means a crash-loop until it is not.