sulthan/mangaBookmark
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects Releases Wiki Activity

Per-Series cover refetch and the store's force path #120

New Issue
Closed
opened 2026-08-17 15:22:44 +07:00 by sulthan · 2 comments
sulthan commented 2026-08-17 15:22:44 +07:00
Owner
Copy Link
Copy Source

Part of #114
Blocked by: #115

Question

How does the owner replace a stale cover for one Series?

This is issue #54, parked since #47's design session (ADR-0007) precisely because there was no
admin dashboard to hang the control off. Its scope as recorded: admin-only, one Series at a time,
re-runs the existing extractor and fetch, no free-text input or upload — a Reader-supplied URL
would put attacker-controlled input into a row every Reader sees, which ADR-0003 closed.

New fact from charting: SetSeriesCover (backend/internal/store/store.go) only fills an empty
cover_address. So this needs a force path in the store, not just a new caller.

To decide:

  • The store surface for a forced replace, and whether it is a new method or a parameter — with
    the constraint that the poll's fill-if-empty behaviour must not become accidentally
    overwriting.
  • What happens to the old bytes in the content-addressed shards (covers.address -> fs path).
    Orphaned files, or a delete, and who is responsible.
  • Whether the address derivation still holds. The content address is the SHA-256 of the source
    URL    , so a Site that re-arts a Series behind the same URL produces the same address — which
    may mean the refetch silently no-ops. This needs checking before the control is specified.
  • Whether the fetch reuses the poller's path (and therefore the browser sidecar for comix and
    kagane covers) or a dedicated one, and how it degrades when the sidecar is unreachable.
  • Whether the action is confirm-gated. It replaces something every Reader sees, but it does not
    pull a Series out of a list.
Part of #114 Blocked by: #115 ## Question How does the owner replace a stale cover for one Series? This is issue #54, parked since #47's design session (ADR-0007) precisely because there was no admin dashboard to hang the control off. Its scope as recorded: admin-only, one Series at a time, re-runs the existing extractor and fetch, no free-text input or upload — a Reader-supplied URL would put attacker-controlled input into a row every Reader sees, which ADR-0003 closed. New fact from charting: `SetSeriesCover` (`backend/internal/store/store.go`) only fills an *empty* `cover_address`. So this needs a force path in the store, not just a new caller. To decide: - The store surface for a forced replace, and whether it is a new method or a parameter — with the constraint that the poll's fill-if-empty behaviour must not become accidentally overwriting. - What happens to the old bytes in the content-addressed shards (`covers.address` -> fs path). Orphaned files, or a delete, and who is responsible. - Whether the address derivation still holds. The content address is the SHA-256 of the *source URL*, so a Site that re-arts a Series behind the same URL produces the same address — which may mean the refetch silently no-ops. This needs checking before the control is specified. - Whether the fetch reuses the poller's path (and therefore the browser sidecar for comix and kagane covers) or a dedicated one, and how it degrades when the sidecar is unreachable. - Whether the action is confirm-gated. It replaces something every Reader sees, but it does not pull a Series out of a list.
sulthan added the wayfinder:grilling label 2026-08-17 15:22:44 +07:00
sulthan commented 2026-08-17 18:57:46 +07:00
Author
Owner
Copy Link
Copy Source

From #119 (closed): the poller command seam is settled — an admin action reaches the Lane as database state (series.force_poll_at, migration 0013), never as a call, and the row action posts to /admin/series/{key}/<verb> answering with the swapped row fragment. If a cover refetch is to run through the Lane, reuse that shape rather than inventing a second one; if it runs synchronously in the request instead, say why the two differ.

From #119 (closed): the poller command seam is settled — an admin action reaches the Lane as database state (`series.force_poll_at`, migration 0013), never as a call, and the row action posts to `/admin/series/{key}/<verb>` answering with the swapped row fragment. If a cover refetch is to run through the Lane, reuse that shape rather than inventing a second one; if it runs synchronously in the request instead, say why the two differ.
sulthan self-assigned this 2026-08-18 07:14:14 +07:00
sulthan commented 2026-08-18 08:46:38 +07:00
Author
Owner
Copy Link
Copy Source

Resolved. The owner replaces a Cover by asking for a Forced Poll — no dedicated control, no second column, no synchronous fetch.

1. Trigger: the Forced Poll carries it. checkOne already holds facts.Cover when it calls fillBlankCover (poller.go:457), so a forced pass costs no extra page read. On a forced pass checkOne takes the replace path; on an unforced pass fill-if-blank is unchanged, and the reason already written at poller.go:70-77 still holds for it — a routine Poll must not spend a cover fetch per Series per cycle or move artwork under a Reader for no visible reason. Rejected: a second series.force_cover_at column (two columns, two verbs, two controls, and another field on #116's projection, for a rare action) and a synchronous fetch inside the admin request (contradicts the map's standing decision that intervention reaches the poller through the database, and would block an owner request on the sidecar). Accepted consequence: the owner cannot refresh a Cover without also re-reading chapters. The two are one act — read this Series page now and accept what it says.

2. Address derivation: new writes hash the bytes; existing rows are left alone; no migration. Three facts make a same-address replace impossible: putCover installs the file with os.Link and ignores fs.ErrExist (store.go:686), the covers insert is ON CONFLICT (address) DO NOTHING (store.go:692), and /covers/{address} is served Cache-Control: public, max-age=604800, immutable (internal/api/handlers.go:175). The address must therefore change for a replace to be visible, and today it is the SHA-256 of the source URL (store.go:619), so a Site that re-arts behind an unchanged URL is invisible twice over — the file is not written and the client would keep the old bytes for a week anyway.

coverAddressRe (^[0-9a-f]{64}$, store.go:718) is the only contract on the value, so a mixed derivation is legal with no schema change: legacy rows keep their URL-derived address and stay readable. Rejected: a rehash of every stored file, which cannot be a migration at all — migrations are SQL-only, globbed by version (store.go:498) — and would need a Go run-once step walking the whole cover tree at boot. Also rejected: a URL hash salted with a per-Series refetch generation (a column whose value means nothing to a reader of the row).

Two deletions come with it. CoverAddress(sourceURL) (store.go:713) has no production caller — only tests and store internals — so the "name a Cover before you have the bytes" property its doc comment defends is not load-bearing. prefetchCover's GetCover(sr.Cover) shortcut (poller.go:109) stops matching byte-addressed rows; delete the shortcut and let that legacy heal path fetch, rather than adding a covers.source_url column to keep an optimisation on a rare repair.

Free consequence, and it closes this ticket's third bullet: the new address equals the old one iff the Site serves the same bytes. "Unchanged" becomes observable instead of a silent no-op, and the page can say so.

The implementing session should write an ADR: the rule is hard to reverse once data exists under both derivations, and it contradicts the URL-hash statement in migrations/0009_series_cover_address.sql and store.go:710-713, which the change must rewrite.

3. The old bytes: nothing is deleted here. Reclamation goes whole to #125, which now owns both causes — a replaced Cover and a removed Orphan Series — rather than this ticket writing one rule and #125 writing another. Notes for that specification: a delete needs a NOT EXISTS guard over series.cover_address, because two Series can share one file; and deleting bytes a live Series still points at is unrepairable, since cover_address is not blank and so neither fillBlankCover (poller.go:83) nor prefetchCover (poller.go:106) heals the row. Measured 2026-08-18 against demonicscans.org: 12 Series from the home page gave 12 distinct cover addresses and 12 distinct byte digests, and a page for a Series that does not exist publishes no og:image at all (so coverFrom, sites.go:356, stores nothing and there is no shared placeholder image). Sharing is rare, not impossible.

4. Fetch path: the existing one. Reuse fetchCoverBytes with fetcherFor routing (cover.go:25-33), so kagane and static.comix.to covers keep going through the sidecar and everything else goes over plain TLS — one routing rule for Acquisition, Poll and this action. No pre-flight refusal when the sidecar is down: the Lane skips its pass, the mark ages, and #119 already made an ageing mark the evidence that a Lane is stuck.

Checked, and no new notification is needed — the surface already exists. lanesView.BrowserConfigured / BrowserReachable (admin.go:38-40) render as Browser sidecar: not configured … / reachable / unreachable (templates/lanes.html:34-40); a Lane that can only be read through the sidecar carries the mark no browser and sets Attention (admin.go:62, admin.go:71); and reachability clears itself after refuseBackoff (status.go:41-59). #117 keeps the fact after deleting LaneStatus(): BrowserConfigured from BROWSER_WS_URL != "" in the web layer's config, reachability derived from a browser Site whose latest pass carries unreachable > 0.

5. The Series row says only how old the request is — #119's check requested <age> ago. It never repeats the Lane-level reason; the Lanes page holds that, per #115's one-figure-in-one-place rule. Rejected: a no browser mark on every Series row of a browser Site, which would put Lane state into the Series list.

6. No confirmation, no --danger, never --ember. A newer Cover is what the Site now publishes; a Cover in Bookmark Manager that disagrees with the Site's is the state that confuses the Reader, so the change is the remedy and not the risk. Cinder's confirm law covers actions that pull a Series out of a list, and this pulls none.

7. Store surface: a second method. ReplaceSeriesCover(site, seriesID, sourceURL, body, contentType) beside SetSeriesCover; both go through putCover and differ only in the UPDATE predicate, which SetSeriesCover keeps as AND cover_address = '' (store.go:752). Rejected a force bool: three existing callers would pass false for ever, and the first caller that passes true turns fill-if-empty into overwrite at the call site rather than in the store — TestSetSeriesCoverDoesNotOverwrite (store_test.go:833) guards behaviour that should stay unparameterised. The forced UPDATE must write cover as well as cover_address, since a re-art may sit behind a new source URL.

Glossary updated in CONTEXT.md: Acquisition no longer claims to be the only read that establishes a Cover, and Forced Poll now states that it takes whatever Cover the Site publishes today.

Resolved. The owner replaces a Cover by asking for a **Forced Poll** — no dedicated control, no second column, no synchronous fetch. **1. Trigger: the Forced Poll carries it.** `checkOne` already holds `facts.Cover` when it calls `fillBlankCover` (poller.go:457), so a forced pass costs no extra page read. On a forced pass `checkOne` takes the replace path; on an unforced pass fill-if-blank is unchanged, and the reason already written at poller.go:70-77 still holds for it — a routine Poll must not spend a cover fetch per Series per cycle or move artwork under a Reader for no visible reason. Rejected: a second `series.force_cover_at` column (two columns, two verbs, two controls, and another field on #116's projection, for a rare action) and a synchronous fetch inside the admin request (contradicts the map's standing decision that intervention reaches the poller through the database, and would block an owner request on the sidecar). Accepted consequence: the owner cannot refresh a Cover without also re-reading chapters. The two are one act — read this Series page now and accept what it says. **2. Address derivation: new writes hash the bytes; existing rows are left alone; no migration.** Three facts make a same-address replace impossible: `putCover` installs the file with `os.Link` and ignores `fs.ErrExist` (store.go:686), the `covers` insert is `ON CONFLICT (address) DO NOTHING` (store.go:692), and `/covers/{address}` is served `Cache-Control: public, max-age=604800, immutable` (internal/api/handlers.go:175). The address must therefore change for a replace to be visible, and today it is the SHA-256 of the *source URL* (store.go:619), so a Site that re-arts behind an unchanged URL is invisible twice over — the file is not written and the client would keep the old bytes for a week anyway. `coverAddressRe` (`^[0-9a-f]{64}$`, store.go:718) is the only contract on the value, so a mixed derivation is legal with no schema change: legacy rows keep their URL-derived address and stay readable. Rejected: a rehash of every stored file, which cannot be a migration at all — migrations are SQL-only, globbed by version (store.go:498) — and would need a Go run-once step walking the whole cover tree at boot. Also rejected: a URL hash salted with a per-Series refetch generation (a column whose value means nothing to a reader of the row). Two deletions come with it. `CoverAddress(sourceURL)` (store.go:713) has no production caller — only tests and `store` internals — so the "name a Cover before you have the bytes" property its doc comment defends is not load-bearing. `prefetchCover`'s `GetCover(sr.Cover)` shortcut (poller.go:109) stops matching byte-addressed rows; delete the shortcut and let that legacy heal path fetch, rather than adding a `covers.source_url` column to keep an optimisation on a rare repair. Free consequence, and it closes this ticket's third bullet: the new address equals the old one **iff** the Site serves the same bytes. "Unchanged" becomes observable instead of a silent no-op, and the page can say so. The implementing session should write an ADR: the rule is hard to reverse once data exists under both derivations, and it contradicts the URL-hash statement in migrations/0009_series_cover_address.sql and store.go:710-713, which the change must rewrite. **3. The old bytes: nothing is deleted here.** Reclamation goes whole to #125, which now owns both causes — a replaced Cover and a removed Orphan Series — rather than this ticket writing one rule and #125 writing another. Notes for that specification: a delete needs a `NOT EXISTS` guard over `series.cover_address`, because two Series can share one file; and deleting bytes a live Series still points at is unrepairable, since `cover_address` is not blank and so neither `fillBlankCover` (poller.go:83) nor `prefetchCover` (poller.go:106) heals the row. Measured 2026-08-18 against demonicscans.org: 12 Series from the home page gave 12 distinct cover addresses and 12 distinct byte digests, and a page for a Series that does not exist publishes no `og:image` at all (so `coverFrom`, sites.go:356, stores nothing and there is no shared placeholder image). Sharing is rare, not impossible. **4. Fetch path: the existing one.** Reuse `fetchCoverBytes` with `fetcherFor` routing (cover.go:25-33), so kagane and `static.comix.to` covers keep going through the sidecar and everything else goes over plain TLS — one routing rule for Acquisition, Poll and this action. No pre-flight refusal when the sidecar is down: the Lane skips its pass, the mark ages, and #119 already made an ageing mark the evidence that a Lane is stuck. Checked, and no new notification is needed — the surface already exists. `lanesView.BrowserConfigured` / `BrowserReachable` (admin.go:38-40) render as `Browser sidecar: not configured … / reachable / unreachable` (templates/lanes.html:34-40); a Lane that can only be read through the sidecar carries the mark `no browser` and sets `Attention` (admin.go:62, admin.go:71); and reachability clears itself after `refuseBackoff` (status.go:41-59). #117 keeps the fact after deleting `LaneStatus()`: `BrowserConfigured` from `BROWSER_WS_URL != ""` in the web layer's config, reachability derived from a browser Site whose latest pass carries `unreachable > 0`. **5. The Series row says only how old the request is** — #119's `check requested <age> ago`. It never repeats the Lane-level reason; the Lanes page holds that, per #115's one-figure-in-one-place rule. Rejected: a `no browser` mark on every Series row of a browser Site, which would put Lane state into the Series list. **6. No confirmation, no `--danger`, never `--ember`.** A newer Cover is what the Site now publishes; a Cover in Bookmark Manager that disagrees with the Site's is the state that confuses the Reader, so the change is the remedy and not the risk. Cinder's confirm law covers actions that pull a Series out of a list, and this pulls none. **7. Store surface: a second method.** `ReplaceSeriesCover(site, seriesID, sourceURL, body, contentType)` beside `SetSeriesCover`; both go through `putCover` and differ only in the UPDATE predicate, which `SetSeriesCover` keeps as `AND cover_address = ''` (store.go:752). Rejected a `force bool`: three existing callers would pass `false` for ever, and the first caller that passes `true` turns fill-if-empty into overwrite at the call site rather than in the store — `TestSetSeriesCoverDoesNotOverwrite` (store_test.go:833) guards behaviour that should stay unparameterised. The forced UPDATE must write `cover` as well as `cover_address`, since a re-art may sit behind a new source URL. Glossary updated in CONTEXT.md: **Acquisition** no longer claims to be the only read that establishes a Cover, and **Forced Poll** now states that it takes whatever Cover the Site publishes today.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Something is broken
 enhancement
New feature or improvement
 needs-info
Waiting on reporter for more information
 needs-triage
Maintainer needs to evaluate
 ready-for-agent
Fully specified, ready for an AFK agent
 ready-for-human
Needs human implementation
 wayfinder:grilling
wayfinder:map
wayfinder:prototype
wayfinder:research
wontfix
Will not be actioned
No Label wayfinder:grilling
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
sulthan
No Assignees sulthan
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sulthan/mangaBookmark#120
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?