diff --git a/.env.example b/.env.example index 61b3f85..e3577f2 100644 --- a/.env.example +++ b/.env.example @@ -9,6 +9,14 @@ API_TOKEN=changeme-generate-a-long-random-token # sites' hostnames change. ALLOWED_ORIGINS=https://asuracomic.net,https://asurascans.com,https://demonicscans.org,https://comix.to,https://kagane.to,https://novelfull.com,https://lightnovelworld.net +# Password for the bundled Postgres container, and therefore half of the +# DATABASE_URL compose builds for the backend. Generate one: +# openssl rand -hex 24 +POSTGRES_PASSWORD=changeme-generate-a-long-random-password + +# Override only to point the backend at a Postgres compose does not run. +# DATABASE_URL=postgres://user:pass@host:5432/bookmarks?sslmode=require + # --- Prod override (Traefik) only --- # Subdomain Traefik routes to this service (required by the prod override). # BOOKMARK_API_HOST=bookmark-api.example.com diff --git a/AGENTS.md b/AGENTS.md index 5779425..7e882d4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,7 @@ Userscript targets **Violentmonkey**, so `GM_*` APIs available, but stay GM-free ``` Two Violentmonkey userscripts (isolated world, per-site adapters, localStorage cache) - -- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> SQLite (volume) + -- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> Postgres (volume) ``` Backend-specific architecture (packages, endpoints, poller, config env vars) lives in `backend/AGENTS.md`. Userscript-specific structure (adapters, retry queue, UI, live URL shapes) lives in `userscript/AGENTS.md`. @@ -33,11 +33,11 @@ Backend-specific architecture (packages, endpoints, poller, config env vars) liv ## Commands Backend (`cd backend`): -- Test all: `go test ./...` +- Test all: `go test ./...` — **needs Docker.** Each test package starts a throwaway `postgres:17-alpine` container (`internal/pgtest`). - Single test: `go test -run TestName ./...` - Build static binary: `CGO_ENABLED=0 go build` -Local stack: `docker compose up` (named volume mounted at `/data`, `restart: unless-stopped`). +Local stack: `docker compose up` (bookmark-api + postgres + headless-shell; `postgres-data` named volume, `restart: unless-stopped`). Smoke test: `curl` endpoints with `Authorization: Bearer `; confirm `OPTIONS` preflight return CORS headers and `/healthz` return 200. @@ -79,7 +79,7 @@ Anchored to OWASP Top 10 / ASVS. Every rule below already has a working example Go backend: -- SQL always parameterized (`?`). Only compile-time constants (`bookmarkColumns`) may be concatenated into query text — never a request value, not even a validated one. +- SQL always parameterized (`$N`). Only compile-time constants (`bookmarkColumns`) may be concatenated into query text — never a request value, not even a validated one. - `html/template` only for anything a browser parses, never `text/template`. Never wrap stored or fetched strings in `template.HTML`/`JS`/`URL`; that switches off the escaping every template depends on. - Any outbound fetch of a client-supplied URL passes `fetchableSeriesURL` (site + `https` + host check) first. `series_url` arrives in a PUT body, so without the gate the poller will probe arbitrary hosts from the server's own network position. New fetch path reuses the gate rather than re-deriving one. - Cap every remote body with `io.LimitReader` (`maxBodyBytes`). An unbounded read is an OOM handed to whatever is on the other end. @@ -133,6 +133,22 @@ Test: "competent reader get this from code in few sec?" Yes → skip. Needs deto `golang-code-style`, `golang-error-handling`, `golang-performance`, `golang-testing` for backend Go work. +## Agent skills + +`AGENTS.md` is the single source of truth for agent guidance; every `CLAUDE.md` in this repo is a symlink to the `AGENTS.md` beside it. Edit `AGENTS.md`. + +### Issue tracker + +Issues live as Gitea issues on `gitea.violetcrown.my.id` (`sulthan/mangaBookmark`), driven by the `tea` CLI — not `gh`. See `docs/agents/issue-tracker.md`. + +### Triage labels + +Default five-role vocabulary, label strings unchanged (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`). See `docs/agents/triage-labels.md`. + +### Domain docs + +Single-context: one root `CONTEXT.md` plus `docs/adr/`, both created lazily. See `docs/agents/domain.md`. + ## graphify Project has knowledge graph at graphify-out/ with god nodes, community structure, cross-file relationships. diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index c3809e1..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,106 +0,0 @@ -# CLAUDE.md - -Guidance for Claude Code (claude.ai/code) working in this repo. - -## What this is - -Manga read-progress tracker, user read on **asurascans.com** (current domain; asuracomic.net 301s here) and **demonicscans.org** via **Violentmonkey**. Userscript inject on-page UI (floating button + slide-in panel), sync progress to self-hosted Go backend so bookmarks unify across both sites and devices. - -## Hard constraints (drive design — don't violate) - -Userscript targets **Violentmonkey**, so `GM_*` APIs available, but stay GM-free where plain web APIs suffice — keeps portability across engines: -- **Avoid `GM_*` unless needed.** Prefer page `localStorage` over `GM_setValue`/`GM_getValue`, on-page UI over `GM_registerMenuCommand`, plain `fetch()` over `GM_xmlhttpRequest` for cross-origin. -- Cross-origin `fetch()` work **only** against CORS-enabled backend. Manga sites `https://`, so backend **must be HTTPS** (else mixed-content block). -- Asura and Demonic are **separate origins with separate `localStorage`** — shared remote store only way to unify bookmarks. Cloud sync required, not optional. -- Userscript run in **isolated world**, so embedded API token safe from site's JS. -- Cloudflare's block on manga sites **IP-reputation-based, not universal — and not reliably reproducible.** Verified 2026-07-26: plain `curl` from both CGNAT dev machine *and* deployed VPS got clean 200s with real HTML on both asurascans.com and demonicscans.org (homepage, series, chapter pages) — no interactive Turnstile challenge from either IP at test time. Contradicts earlier untested assumption CGNAT dev IP blocked; wasn't, at least this date. Treat "does curl work right now" as live, time-varying fact to re-check, not fixed property of machine — Cloudflare's bot scoring can flip previously-clean IP without notice. Backend fetcher still needs graceful-degrade path for when challenged, and adapters should be **verified against live pages** (Playwright MCP, on-device devtools, or direct probe) before finalize, not assumed from single earlier test. - -## Architecture - -``` -Violentmonkey userscript (isolated world, per-site adapters, localStorage cache) - -- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> SQLite (volume) -``` - -Backend-specific architecture (packages, endpoints, poller, config env vars) lives in `backend/CLAUDE.md`. Userscript-specific structure (adapters, retry queue, UI, live URL shapes) lives in `userscript/CLAUDE.md`. - -## Commands - -Backend (`cd backend`): -- Test all: `go test ./...` -- Single test: `go test -run TestName ./...` -- Build static binary: `CGO_ENABLED=0 go build` - -Local stack: `docker compose up` (named volume mounted at `/data`, `restart: unless-stopped`). - -Smoke test: `curl` endpoints with `Authorization: Bearer `; confirm `OPTIONS` preflight return CORS headers and `/healthz` return 200. - -## Forge: Gitea, not GitHub - -`origin` is self-hosted Gitea instance (`gitea.violetcrown.my.id`), so **`gh` don't work here — use `tea` (Gitea CLI) for anything past plain git.** Common ones: - -- Open PR: `tea pr create --head --base main --title "..." --description "..."` -- List / view / check out: `tea pr list`, `tea pr `, `tea pr checkout ` -- Issues: `tea issue create`, `tea issue list` -- Auth lives in `tea login`, not `GH_TOKEN` env var. - -`tea` print output as rendered boxes rather than plain text; PR URL lands on last line. - -## Design system - -Web UI + userscript panel follow **Cinder**, rules in `docs/design-system.md` -— source of truth Claude Design project `BookmarkManager Web UI` -(`969ac210-fe02-4c01-ae1b-9a271dcc779a`). Read it before touching -`backend/internal/web/static/style.css`, `backend/internal/web/templates/*`, or userscript -`TEMPLATE`/`CSS`. Core law: **ember means new chapter only** — no other -state (busy, error, destruction) may use `--ember`; destruction gets -`--danger`. No cards/corners/shadows, one `--measure: 760px` column, tokens -only (never hardcode hex outside `:root`), both colour branches touched -together. Any move that pulls series out of list (archive/finish/remove) -must be confirm-gated via its own `.confirm-row`; only restore fires -instantly. - -## Security invariants - -- Auth on `/bookmarks*`: require `Authorization: Bearer `, **constant-time compare**, 401 otherwise. -- CORS: reflect `Origin` only when in `ALLOWED_ORIGINS`; allow `GET,PUT,DELETE,OPTIONS` + headers `Authorization,Content-Type`; answer preflight `OPTIONS` with `204`. - -## Comments - -Comment only if code alone can't carry info. Cost per read — must earn spot. - -Write for: -- Why not what. Tradeoffs, non-obvious decisions. -- Load-bearing detail looking incidental — say so if "simplify" breaks it. -- Non-local consequence, invisible from function alone. -- Wire format / encoding / interface contract — save callers re-deriving. -- Gotcha/workaround, with ref if exists. -- Domain/business rule not derivable from code. - -Skip: -- Restating code (no `// increment i` above `i++`). -- Trivial getter/setter/pass-through. -- Banners, dividers, `// helpers`. -- Change narration (`// fix bug`, `// as requested`, `// new impl`) — git's job. -- Commented-out code — delete. -- TODO without concrete action. - -Style: one dense comment over function beats one per line inside. Tight, no worked example unless bug subtle. Wrong comment worse than none — update/delete on change. Default fewer — sparse+high-signal beats comprehensive. - -Test: "competent reader get this from code in few sec?" Yes → skip. Needs detour through another file/spec/git-blame → write it. - -## Relevant skills - -`multi-stage-dockerfile` and `docker-compose-orchestration` for container work (referenced in plan). - -`golang-code-style`, `golang-error-handling`, `golang-performance`, `golang-testing` for backend Go work. - -## graphify - -Project has knowledge graph at graphify-out/ with god nodes, community structure, cross-file relationships. - -Rules: -- For codebase questions, first run `graphify query ""` when graphify-out/graph.json exists. Use `graphify path "" ""` for relationships and `graphify explain ""` for focused concepts. Return scoped subgraph, usually much smaller than GRAPH_REPORT.md or raw grep output. -- If graphify-out/wiki/index.md exists, use for broad navigation instead of raw source browsing. -- Read graphify-out/GRAPH_REPORT.md only for broad architecture review or when query/path/explain don't surface enough context. -- After modifying code, run `graphify update .` to keep graph current (AST-only, no API cost). \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..1fc437b --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,69 @@ +# Bookmark Manager + +Read-progress tracker for serialised fiction. A reader browses third-party manga and +novel sites; userscripts capture where they got to and sync it to a self-hosted backend, +so progress survives across sites and devices. + +## Language + +**Series**: +One ongoing work — a manga or a novel — as published by a Site. Identified by its +stable slug on that Site, never by its title. A Series exists once and is shared by +every Reader who bookmarks it; it owns the facts that are true regardless of who is +reading — title, cover, Latest Chapter. A Reader cannot change them; they describe the +Series, not anyone's relationship to it. +_Avoid_: manga, title, book, comic + +**Site**: +One third-party source a Series is published on. A Series on two Sites is two Series. +_Avoid_: source, host, provider, domain + +**Reader**: +A person with their own Progress. Exactly one per set of credentials, so there is no +separate "account" concept to model — the credential belongs to the Reader. +_Avoid_: user, account, member, subscriber + +**Bookmark**: +One Reader's tracked relationship with one Series, holding only what differs between +Readers: Progress, Favourite, Lifecycle bucket. Facts about the Series itself belong +to the Series, not here. +_Avoid_: entry, item, record, subscription + +**Library**: +One of the two halves of the collection — manga or novel — selected by a Bookmark's +`kind`. The web UI and the userscripts each address exactly one Library at a time. +Not a per-person concept: "everything one person has bookmarked" is a different idea +and must not be called a Library. +_Avoid_: section, tab, category + +**Progress**: +The furthest chapter a reader has actually read in a Series. Only a change in Progress +is real activity, so only Progress reorders the list. +_Avoid_: position, bookmark (the noun is taken), last read + +**Latest Chapter**: +The newest chapter a Site has published for a Series, discovered without the reader +present. Distinct from Progress in every way that matters: it is a fact about the Site, +not about the reader, and it must never reorder the list. +_Avoid_: newest, current chapter, update + +**Poll**: +The backend's own check of a Site for a Series's Latest Chapter, made without the +Reader present. Performed once per Series no matter how many Readers bookmarked it — +a Poll is work done on behalf of the Series, never on behalf of a Reader. +_Avoid_: scrape, refresh, check, sync + +**New Chapter**: +The state where Latest Chapter is ahead of Progress. The single condition the ember +accent is permitted to signal. +_Avoid_: unread, update available + +**Lifecycle bucket**: +Which of three mutually exclusive states a Bookmark sits in — reading, archived, or +finished. A Bookmark is in exactly one. Orthogonal to being a favourite. +_Avoid_: state, status (as a domain word), list + +**Favourite**: +A reader's manual pin on a Bookmark. Orthogonal to the Lifecycle bucket, and never a +reason to reorder the list. +_Avoid_: starred, pinned, priority diff --git a/DEPLOY.md b/DEPLOY.md index b5911e9..762bcb4 100644 --- a/DEPLOY.md +++ b/DEPLOY.md @@ -38,6 +38,14 @@ API_TOKEN= # CORS allowlist — leave as-is unless a site changes hostname. ALLOWED_ORIGINS=https://asuracomic.net,https://asurascans.com,https://demonicscans.org,https://comix.to,https://kagane.to +# Required — password for the bundled Postgres container. Compose builds the +# backend's DATABASE_URL out of it and has no fallback for either. +POSTGRES_PASSWORD= + +# Leave unset. Only set this to point the backend at a Postgres compose does +# not run; it then replaces the URL built from POSTGRES_PASSWORD above. +# DATABASE_URL=postgres://user:pass@host:5432/bookmarks?sslmode=require + # Required for the Traefik override. Both have no fallback — compose refuses # to start without them. BOOKMARK_WEB_HOST is required even if you never set # WEB_PASSWORD; see 1b. @@ -50,13 +58,20 @@ BOOKMARK_WEB_HOST=bookmark.violetcrown.my.id # TRAEFIK_CERTRESOLVER=le ``` -Generate + insert the token in one line: +Generate + insert the two secrets in three lines: ```bash sed -i "s|^API_TOKEN=.*|API_TOKEN=$(openssl rand -hex 32)|" .env +sed -i "s|^POSTGRES_PASSWORD=.*|POSTGRES_PASSWORD=$(openssl rand -hex 24)|" .env grep -E '^API_TOKEN=' .env # copy this — the userscript needs the same value ``` +`POSTGRES_PASSWORD` is read **only while the `postgres-data` volume is empty**, +which in practice means at first boot. Changing it afterwards changes the URL +the backend dials but not the password the database expects, and `bookmark-api` +crash-loops on `password authentication failed`. Set it before §2 and leave it +alone. + > Match `TRAEFIK_ENTRYPOINT` / `TRAEFIK_CERTRESOLVER` to your Traefik's actual > names (check your Traefik static config — common alternatives: `https`, > `myresolver`, `cloudflare`). Wrong names = no certificate issued. @@ -116,16 +131,21 @@ This merges the base file (build/image/env/volume) with the prod override (no host port, Traefik network + router labels). Always pass **both** `-f` flags — the prod file is not standalone. -Two services come up: `bookmark-api` (the backend) and `headless-shell`, a CDP -sidecar the poller uses to fetch kagane (behind a Cloudflare JS challenge). -It has no published port — only `bookmark-api` can reach it, over -`BROWSER_WS_URL`. Missing or unreachable, the poller just skips kagane and -logs it; nothing else is affected. +Three services come up: `bookmark-api` (the backend), `postgres` (its database, +`postgres:17-alpine`), and `headless-shell`, a CDP sidecar the poller uses to +fetch kagane (behind a Cloudflare JS challenge). Neither of the latter two +publishes a port: `postgres` sits alone with `bookmark-api` on an +`internal: true` network, and `headless-shell` is reachable only over +`BROWSER_WS_URL`. A missing headless-shell just makes the poller skip kagane and +log it. A missing Postgres stops everything — `bookmark-api` waits for +`pg_isready` to pass, then applies its embedded migrations, and only then +listens. The schema is created that way; there is nothing to import by hand. Check it's up and healthy: ```bash docker compose -f docker-compose.yml -f docker-compose.prod.yml ps +# bookmark-api Up; postgres Up (healthy) docker logs bookmark-api --tail 20 # expect: "listening on :8080 ..." ``` @@ -214,7 +234,10 @@ Pull new code, then rebuild: docker compose -f docker-compose.yml -f docker-compose.prod.yml up -d --build ``` -SQLite data persists in the named volume `bookmarks-data` across rebuilds. +Data persists in the named volume `postgres-data` across rebuilds. (If this +server predates the Postgres migration, the old SQLite volume `bookmarks-data` +is still on disk and deliberately undeclared in compose so `down -v` cannot take +it; see `REDEPLOY.md` §1 for when to remove it.) --- @@ -227,7 +250,9 @@ SQLite data persists in the named volume `bookmarks-data` across rebuilds. | `fetch` fails in the userscript, `curl` works | Origin missing from `ALLOWED_ORIGINS`, or mixed content (backend not HTTPS). | | 401 with the right token | Trailing space/newline in `API_TOKEN`; regenerate and restart. | | Panel button absent | URL didn't match an adapter, or user scripts disabled in Bromite. | -| `compose ... config` errors about `API_TOKEN` | Run compose from the dir with `.env`, or export the vars. | +| `compose ... config` errors about `API_TOKEN` or `POSTGRES_PASSWORD` | Run compose from the dir with `.env`, or export the vars. Both are required and neither has a fallback. | +| `bookmark-api` restarts in a loop, `password authentication failed for user "bookmarks"` | `POSTGRES_PASSWORD` was changed after first boot; Postgres only applies it to an empty `postgres-data`. Restore the old value, or reset the role (`REDEPLOY.md` troubleshooting). | +| `bookmark-api` never logs `listening on :8080` | It is blocked on `postgres` passing `pg_isready`, or a migration failed. `docker compose -f docker-compose.yml -f docker-compose.prod.yml logs postgres`. | Backend config reference and endpoint list: see `README.md`. diff --git a/README.md b/README.md index 2174af3..fa2c67c 100644 --- a/README.md +++ b/README.md @@ -7,14 +7,14 @@ all four sites and all devices. Two parts: -- **`backend/`** — tiny Go (`net/http` + pure-Go SQLite) sync service. 4 routes, +- **`backend/`** — tiny Go (`net/http` + Postgres via pure-Go `pgx`) sync service. 4 routes, static binary, distroless container. - **`userscript/manga-bookmark.user.js`** — single Bromite-compatible userscript (no `GM_*` APIs) that injects an on-page bookmark UI and syncs via `fetch()`. ``` Bromite userscript (isolated world, Shadow DOM UI, localStorage cache) - -- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> SQLite (volume) + -- fetch() HTTPS --> reverse proxy (TLS + CORS) --> Go net/http --> Postgres (volume) ``` --- @@ -27,7 +27,7 @@ Bromite userscript (isolated world, Shadow DOM UI, localStorage cache) |-----|---------|-------| | `API_TOKEN` | *(required)* | Bearer token shared with the userscript. | | `ALLOWED_ORIGINS` | Asura + Demonic + Comix + Kagane origins | Comma-separated CORS allowlist. | -| `DB_PATH` | `/data/bookmarks.db` | SQLite file location. | +| `DATABASE_URL` | *(required)* | Postgres connection URL, e.g. `postgres://bookmarks:…@postgres:5432/bookmarks?sslmode=disable`. Compose builds it from `POSTGRES_PASSWORD`. | | `PORT` | `8080` | Plain HTTP; TLS terminated by the proxy. | | `BROWSER_WS_URL` | `ws://172.28.0.10:9222` | Headless-shell CDP endpoint used to poll Kagane past its JS challenge. Must be an IP or `localhost` — Chrome's DevTools handler 500s any other Host header. | @@ -60,11 +60,17 @@ go test ./... # unit + handler tests CGO_ENABLED=0 go build # static binary ``` +**`go test ./...` requires Docker.** The store talks to a real Postgres, so +each test package starts a throwaway `postgres:17-alpine` container and gives +every test its own database inside it (`internal/pgtest`). Nothing is stubbed +and nothing reaches the network beyond the local Docker daemon. + ### Run the stack ```bash cp .env.example .env -# edit .env: set API_TOKEN (openssl rand -hex 32) +# edit .env: set API_TOKEN (openssl rand -hex 32) and +# POSTGRES_PASSWORD (openssl rand -hex 24) docker compose up -d --build # binds 127.0.0.1:8080 ``` diff --git a/REDEPLOY.md b/REDEPLOY.md index 9a714c1..5118915 100644 --- a/REDEPLOY.md +++ b/REDEPLOY.md @@ -18,7 +18,7 @@ the checkout cannot take the backups with them. ``` /opt/ ├── bookmarkmanager/ <- the checkout (this repo) -└── bookmarkmanager-backups/ <- bookmarks-YYYYmmdd-HHMMSS.db +└── bookmarkmanager-backups/ <- bookmarks-YYYYmmdd-HHMMSS.dump ``` --- @@ -53,78 +53,97 @@ echo "$BACKUP_DIR" # -> /opt/bookmarkmanager-backups ## 1. Back up the database -The database is a single SQLite file in the named Docker volume, at -`/data/bookmarks.db` inside the container. Find the volume's real name — Compose -prefixes it with the project directory: +The database is Postgres, running as the `postgres` service on the named volume +`postgres-data`. It has **no published port** — nothing outside the internal `db` +network can reach it — so every command below goes in through the container: ```bash -docker volume ls --filter name=bookmarks-data -# -> local bookmarkmanager_bookmarks-data -VOL=$(docker volume ls --filter name=bookmarks-data -q | head -1) +$COMPOSE exec -T postgres psql -U bookmarks -d bookmarks -c '\dt' +# -> bookmarks, schema_migrations ``` -### Preferred: hot backup, no downtime +Inside the container that connects over the local socket as the `bookmarks` +superuser, so no password is needed anywhere in this section. `-T` is not +optional: without it Compose allocates a TTY, which rewrites `\n` to `\r\n` and +silently corrupts any binary stream flowing back out — see the dump below. -The store runs in **WAL mode**, so recent writes may still be sitting in -`bookmarks.db-wal`. Copying `bookmarks.db` alone while the container runs can -therefore silently drop the newest bookmarks. `VACUUM INTO` folds the WAL in and -writes one consistent file, safe to run against a live database: +### Preferred: hot dump, no downtime + +`pg_dump` runs in a single repeatable-read transaction, so it writes one +point-in-time-consistent snapshot while the API keeps serving. No stopping, no +WAL to worry about — that is the server's problem, not yours. ```bash STAMP=$(date -u +%Y%m%d-%H%M%S) # UTC, sorts chronologically as text -docker run --rm \ - -v "$VOL":/data \ - -v "$BACKUP_DIR":/backup \ - alpine sh -c "apk add -q sqlite && - sqlite3 /data/bookmarks.db \"VACUUM INTO '/backup/bookmarks-$STAMP.db'\"" +$COMPOSE exec -T postgres pg_dump -U bookmarks -d bookmarks -Fc \ + > "$BACKUP_DIR/bookmarks-$STAMP.dump" -ls -lh "$BACKUP_DIR"/bookmarks-$STAMP.db +ls -lh "$BACKUP_DIR"/bookmarks-$STAMP.dump ``` -`$STAMP` is the "time in the name" — `bookmarks-20260730-014233.db`. UTC, so the -files sort in real order and never collide across a DST shift. +`-Fc` is the custom archive format rather than plain SQL: it is compressed, and +`pg_restore` can inspect and replay it selectively — list its table of contents, +restore one table, restore schema without data, reorder. A plain `.sql` dump can +only be piped into `psql` whole, and gives you no way to check what is in it +short of reading it. -Note the source volume is mounted **read-write**, which looks wrong for a backup -and is not. Opening a WAL database requires creating the `-shm` shared-memory -file; with `:ro` the command fails with `unable to open database file` and no -backup is produced. `VACUUM INTO` never writes to the source itself. +`$STAMP` is the "time in the name" — `bookmarks-20260730-014233.dump`. UTC, so +the files sort in real order and never collide across a DST shift. Verify it before you trust it. An unreadable backup is worse than none, because you will act as though you have one: ```bash -docker run --rm -v "$BACKUP_DIR":/backup alpine sh -c "apk add -q sqlite && - sqlite3 /backup/bookmarks-$STAMP.db 'PRAGMA integrity_check;' && - sqlite3 /backup/bookmarks-$STAMP.db 'SELECT count(*) FROM bookmarks;'" -# -> ok +# 1. The dump parses and contains the tables. Uses the same image compose +# already pulls, so nothing new to install. +docker run --rm -v "$BACKUP_DIR":/backup postgres:17-alpine \ + pg_restore --list "/backup/bookmarks-$STAMP.dump" | grep 'TABLE DATA' +# -> 1234; 0 0 TABLE DATA public bookmarks bookmarks +# -> 1235; 0 0 TABLE DATA public schema_migrations bookmarks + +# 2. Sanity-check the live row count you just captured. +$COMPOSE exec -T postgres psql -U bookmarks -d bookmarks \ + -c 'select count(*) from bookmarks' # -> 37 ``` -The count should match what the web UI shows. Zero rows on a server you know has -bookmarks means you backed up the wrong volume. +A custom-format archive stores row counts nowhere, so step 1 proves the file is +a readable archive with the right tables in it, not that the rows are there; +step 2 is the number those rows should be. It should match what the web UI +shows. Zero on a server you know has bookmarks means the API and your `psql` +are looking at different databases — check `DATABASE_URL`. -### Fallback: cold copy (no network for `apk add sqlite`) +### Fallback: cold volume archive -Stop the service first, then copy the database **and its sidecars** — the `-wal` -is not optional, it is where the newest writes are: +Use this when you want the whole data directory rather than a logical dump — a +like-for-like restore of the same Postgres major version onto the same host. + +**The stack must be stopped first.** A running Postgres has dirty pages in +shared buffers and WAL that has not been replayed into the data files, and `tar` +walks the directory over several seconds while the server keeps writing to it. +The archive you get is torn: files from different instants, possibly a +half-written page. It may restore, start, and be quietly wrong. Online +filesystem-level backup is `pg_basebackup`'s job, not `tar`'s; with the +container stopped the shutdown checkpoint has already flushed everything and a +plain archive of the volume is consistent. ```bash +VOL=$(docker volume ls --filter name=postgres-data -q | head -1) +echo "$VOL" # -> bookmarkmanager_postgres-data + $COMPOSE stop -docker run --rm -v "$VOL":/data:ro -v "$BACKUP_DIR":/backup alpine sh -c " - cp /data/bookmarks.db /backup/bookmarks-$STAMP.db - [ -f /data/bookmarks.db-wal ] && cp /data/bookmarks.db-wal /backup/bookmarks-$STAMP.db-wal - [ -f /data/bookmarks.db-shm ] && cp /data/bookmarks.db-shm /backup/bookmarks-$STAMP.db-shm - ls -1 /backup" +docker run --rm -v "$VOL":/from:ro -v "$BACKUP_DIR":/to alpine \ + tar czf "/to/postgres-data-$STAMP.tgz" -C /from . $COMPOSE start + +ls -lh "$BACKUP_DIR"/postgres-data-$STAMP.tgz ``` -Costs ~10 seconds of downtime. A clean shutdown usually checkpoints the WAL away, -so seeing only the `.db` file is normal and fine — the `[ -f ]` guards exist for -the case where it did not. Restoring this variant means putting whichever files -you got back together, under their original names. - -Read-only is safe here precisely because nothing opens the database: it is a file -copy, not a SQLite connection. +Costs ~15 seconds of downtime. Read-only on the source is safe here precisely +because nothing is running against it. Restoring this variant means untarring it +back into an *empty* `postgres-data` volume with the stack down — it is a whole +data directory, not a file you can drop next to the live one, and it will only +start under `postgres:17`. ### Retention @@ -132,7 +151,19 @@ Keep a month, drop the rest — a bookmark database this small compresses the decision to "disk is free, but not infinite": ```bash -ls -1t "$BACKUP_DIR"/bookmarks-*.db | tail -n +31 | xargs -r rm -v +ls -1t "$BACKUP_DIR"/bookmarks-*.dump | tail -n +31 | xargs -r rm -v +``` + +### A note on the old `bookmarks-data` volume + +`bookmarks-data` is the **pre-migration SQLite volume**. It is deliberately not +declared in `docker-compose.yml` any more, which is what keeps `docker compose +down -v` from taking it with the rest of the stack. It is not the live database +and nothing reads it. Once the Postgres data has been trusted for a while, +remove it by hand — nothing else will: + +```bash +docker volume rm bookmarkmanager_bookmarks-data ``` --- @@ -171,11 +202,15 @@ rebuilt. The one exception is `userscript/manga-bookmark.user.js`, which is bindmounted read-only and read fresh per request. ```bash -$COMPOSE ps # Up, and recently (re)created +$COMPOSE ps # bookmark-api Up; postgres Up (healthy) docker logs bookmark-api --tail 20 # -> "listening on :8080 ..." ``` -Nothing in the log about the database or the poller failing. The image is tagged +Nothing in the log about the database, the migrations or the poller failing. +`bookmark-api` waits on `postgres` reporting healthy before it starts and the +binary applies any pending migration before it listens, so an API that never +says "listening" is usually the database, not the code — `$COMPOSE logs +postgres` first. The image is tagged `bookmarkmanager-backend:latest`, so the previous image is still on disk untagged — that is what makes the rollback in §6 quick. @@ -199,9 +234,16 @@ curl -s -i -X OPTIONS -H 'Origin: https://asurascans.com' \ $API/bookmarks/x | grep -i access-control # -> allow-origin echoed ``` -`[]` from the third call is the alarm that matters: the volume is not attached -and you are looking at an empty database. Stop and check `$COMPOSE config ---volumes` before touching anything else. +`[]` from the third call is the alarm that matters: you are talking to an empty +database, which means the API found a *different* Postgres than the one holding +your data — a renamed project directory, a fresh `postgres-data`, or a +`DATABASE_URL` override in `.env` pointing elsewhere. Stop and check, before +touching anything else: + +```bash +$COMPOSE config --volumes # -> postgres-data +$COMPOSE exec -T postgres psql -U bookmarks -d bookmarks -c 'select count(*) from bookmarks' +``` Web UI and its assets: @@ -267,33 +309,41 @@ git checkout $COMPOSE up -d --build ``` -**Database damaged** — restore the backup from §1. Stop first: the running -process holds the WAL, and dropping a file under a live SQLite connection -corrupts what you were trying to save. +**Database damaged** — restore the dump from §1. Stop **only the API**, not the +whole stack: `pg_restore` needs the server up to restore into, and it needs +`bookmark-api`'s connection pool gone, because `--clean` cannot drop a table +other sessions are holding open. ```bash -$COMPOSE stop +$COMPOSE stop bookmark-api -docker run --rm -v "$VOL":/data -v "$BACKUP_DIR":/backup alpine sh -c ' - rm -f /data/bookmarks.db /data/bookmarks.db-wal /data/bookmarks.db-shm && - cp /backup/bookmarks-.db /data/bookmarks.db && - chown 65532:65532 /data/bookmarks.db && - ls -l /data' +$COMPOSE exec -T postgres pg_restore -U bookmarks -d bookmarks --clean --if-exists \ + < "$BACKUP_DIR/bookmarks-.dump" -$COMPOSE start +$COMPOSE start bookmark-api docker logs bookmark-api --tail 20 curl -s -H "Authorization: Bearer $TOKEN" $API/bookmarks | head -c 200 ``` -Two steps here are easy to skip and both bite: +Three things here are easy to skip and all three bite: -- **Delete the stale `-wal` and `-shm`.** Leaving them beside a restored database - mixes two different histories; SQLite will either refuse to open it or quietly - reapply writes you meant to discard. -- **`chown 65532:65532`.** The image is `distroless/static:nonroot` and runs as - that uid, while the helper container above writes as root. A root-owned - database opens read-only-ish: reads work, so `/bookmarks` looks fine, and then - every write fails. That is the worst possible failure mode — it looks restored. +- **`--clean --if-exists`.** Without `--clean` the dump's rows land *on top of* + what is already there and you get primary-key collisions half way through, a + partially restored database, and a non-zero exit you may not notice. + `--if-exists` only suppresses the "does not exist" noise when the target is + already empty; it is not the part doing the work. +- **`-T` again.** Feeding a custom-format archive into a TTY-allocated `exec` + corrupts it in flight and `pg_restore` fails with a garbled-header error on a + file that is perfectly fine on disk. +- **Stop the API, not Postgres.** `$COMPOSE stop` (everything) leaves you with + nothing to restore into; leaving `bookmark-api` running leaves connections + that block the drops *and* lets the poller write into a half-restored table. + +No ownership fixing is needed any more — the Postgres image owns `postgres-data` +itself and `pg_restore` writes through the server, not the filesystem. +`schema_migrations` is inside the dump, so the database comes back at whatever +schema version the backup was taken at; the migration runner applies anything +newer the next time `bookmark-api` starts. --- @@ -305,21 +355,24 @@ For a routine redeploy where nothing needs deciding: cd /opt/bookmarkmanager COMPOSE="docker compose -f docker-compose.yml -f docker-compose.prod.yml" BACKUP_DIR="$(cd .. && pwd)/bookmarkmanager-backups"; mkdir -p "$BACKUP_DIR" -VOL=$(docker volume ls --filter name=bookmarks-data -q | head -1) STAMP=$(date -u +%Y%m%d-%H%M%S) -docker run --rm -v "$VOL":/data -v "$BACKUP_DIR":/backup alpine sh -c \ - "apk add -q sqlite && sqlite3 /data/bookmarks.db \"VACUUM INTO '/backup/bookmarks-$STAMP.db'\" && - sqlite3 /backup/bookmarks-$STAMP.db 'PRAGMA integrity_check;'" && +$COMPOSE exec -T postgres pg_dump -U bookmarks -d bookmarks -Fc \ + > "$BACKUP_DIR/bookmarks-$STAMP.dump" && +docker run --rm -v "$BACKUP_DIR":/backup postgres:17-alpine \ + pg_restore --list "/backup/bookmarks-$STAMP.dump" > /dev/null && git pull --ff-only && $COMPOSE up -d --build && sleep 5 && curl -sf https://bookmark-api.violetcrown.my.id/healthz && echo " deploy ok" ``` -The `&&` chain is deliberate: if the backup or its integrity check fails, -nothing is pulled and nothing is rebuilt. Then still do §5 by hand — no shell -command can tell you the panel works on the phone. +The `&&` chain is deliberate: if the dump or its `pg_restore --list` check +fails, nothing is pulled and nothing is rebuilt. A failed dump still leaves a +short or empty `.dump` behind — the shell creates the file before `pg_dump` +runs — so delete it rather than letting it sit in the backup directory looking +like a backup. Then still do §5 by hand — no shell command can tell you the +panel works on the phone. --- @@ -327,16 +380,18 @@ command can tell you the panel works on the phone. | Symptom | Cause / fix | |---|---| -| `/bookmarks` returns `[]` after redeploy | Volume not attached — check `$COMPOSE config --volumes` and that you passed both `-f` files. Do **not** re-bookmark; the data is still in the volume. | +| `/bookmarks` returns `[]` after redeploy | You are on an empty Postgres. Check `$COMPOSE config --volumes` lists `postgres-data`, that you passed both `-f` files, and that `.env` has no stray `DATABASE_URL` override. Do **not** re-bookmark; the data is still in the volume. | | UI looks like plain Georgia / system sans | `static/fonts/` missing from the image, or the browser cached an old `style.css`. `/static/*` is served `max-age=3600`, so hard-reload or wait an hour. | | CSS or template change did not appear | You restarted without `--build`. Assets are `//go:embed`ed. | | Font answers `application/octet-stream` | Old binary — the `.woff2` MIME registration is in `web.go`. Rebuild. | | Everyone logged out of the web UI | `API_TOKEN` or `WEB_PASSWORD` changed; sessions are derived from both. Expected, just log in again. | | `compose` errors about `BOOKMARK_WEB_HOST` | Run from the directory holding `.env`. Both host vars are required even when the web UI is unused. | | Userscript did not update on the phone | Violentmonkey polls on its own schedule; force a check. `@version` comes from the file's mtime, so confirm the pull actually touched it. | -| `apk add sqlite` fails (no network) | Use the cold-copy fallback in §1 — and copy `bookmarks.db-wal` too. | -| Reads work but every write fails after a restore | Restored file is root-owned; the container is uid 65532. `chown 65532:65532` it (§6). | -| Backup command: `unable to open database file` | Source volume mounted `:ro`. WAL needs to create `-shm`; mount it read-write (§1). | +| `bookmark-api` crash-loops, log says `password authentication failed for user "bookmarks"` | `POSTGRES_PASSWORD` in `.env` no longer matches the one burned into `postgres-data` at first init — Postgres reads that variable only when initialising an empty volume. Put the old value back, or reset the role: `$COMPOSE exec postgres psql -U bookmarks -d bookmarks -c '\password bookmarks'` (prompts, so nothing lands in shell history) and then match `.env` to it. | +| `compose` errors `set POSTGRES_PASSWORD in .env` | Unset. Compose builds the backend's `DATABASE_URL` out of it, so it is required even though you never write that URL yourself. Run from the directory holding `.env`. | +| `postgres` never leaves `starting`; `bookmark-api` never starts either | The healthcheck (`pg_isready`) is failing and `bookmark-api` waits on it. `$COMPOSE logs postgres` — usually `postgres-data` was initialised by a different major version ("database files are incompatible with server"), or the disk is full. | +| `pg_restore`: `cannot drop … other objects depend on it` / `being accessed by other users` | Live connections block `--clean`. `$COMPOSE stop bookmark-api` first (§6). If they persist: `$COMPOSE exec -T postgres psql -U bookmarks -d postgres -c "select pg_terminate_backend(pid) from pg_stat_activity where datname='bookmarks' and pid <> pg_backend_pid()"`. | +| Dump is 0 bytes, or `pg_restore`: `did not find magic string in file header` | You ran `exec` without `-T`. The allocated TTY rewrites newlines in the binary stream and corrupts the archive in flight (§1). | Full first-time setup: `DEPLOY.md`. Config reference and endpoints: `README.md`. UI conventions: `docs/design-system.md`. diff --git a/backend/AGENTS.md b/backend/AGENTS.md index c29bdb4..d682c1c 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -1,8 +1,8 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGENTS.md` for the project-wide architecture diagram, hard constraints, and design system. -- **Backend** (`backend/`): stdlib `net/http` (handful routes, no framework) + `modernc.org/sqlite` (pure Go, `CGO_ENABLED=0` -> static binary -> distroless/scratch image). Reverse proxy terminates TLS; Go service listens plain `:8080`. +- **Backend** (`backend/`): stdlib `net/http` (handful routes, no framework) + Postgres over `jackc/pgx/v5` (pure Go, `CGO_ENABLED=0` -> static binary -> distroless/scratch image). Reverse proxy terminates TLS; Go service listens plain `:8080`. Single binary, split into packages under `backend/internal/`: `store` - (Bookmark type, SQLite persistence, migrations), `latest` (background + (Bookmark type, Postgres persistence, migration runner), `latest` (background poller, site parsers, TLS fetcher), `session` (cookie signing, login rate limiter), `httpmw` (Auth/Gzip/CORS middleware), `api` (JSON bookmark handlers), `userscript` (userscript-serving handler), `web` @@ -11,6 +11,16 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN packages together into `newRouter`. Root-level `*_test.go` hold integration tests that exercise the full router; unit tests for a package live beside it under `internal/`. +- **Schema is migration-owned.** `internal/store/migrations/*.sql` is + `go:embed`-ed and applied on every start by `store.migrate`: one numbered + file per change, one transaction each, versions recorded in + `schema_migrations`. Files are **append-only** — editing an applied one + changes nothing on a database that already ran it. No column probing, no + data-fixup migrations: both were SQLite-era machinery and are gone. +- **Tests need Docker.** `internal/pgtest` starts one `postgres:17-alpine` + container per test binary (`TestMain` -> `pgtest.Main`) and hands each test + its own database (`pgtest.URL(t)`). A package whose tests touch the store + must have that `TestMain`. - **Single-user store.** One `bookmarks` table keyed `:` (`asura`|`demonic`|`comix`|`kagane`|`novelfull`|`lightnovelworld`), with a `kind` column (`manga`|`novel`) splitting the two libraries. Sync **last-write-wins**. Schema and endpoint list in plan. - **Endpoints:** `GET /bookmarks`, `PUT /bookmarks/{key}` (upsert; see `updated_at` rule below), `DELETE /bookmarks/{key}`, `GET /healthz` (no auth). - **Web UI:** same binary serve password-gated browser UI on second @@ -70,8 +80,9 @@ Guidance for OpenCode (and Claude Code) working under `backend/`. See root `AGEN `excluded.*` is post-evaluation row and default applied there would wipe bucket on every PUT from client that predates column. See `docs/superpowers/specs/2026-07-27-status-buckets-design.md`. -- **Config via env:** `API_TOKEN`, `ALLOWED_ORIGINS` (comma list), `DB_PATH` - (default `/data/bookmarks.db`), `PORT` (default `8080`), `WEB_PASSWORD` +- **Config via env:** `API_TOKEN`, `ALLOWED_ORIGINS` (comma list), + `DATABASE_URL` (Postgres connection URL, required — no default), + `PORT` (default `8080`), `WEB_PASSWORD` (gates browser UI; unset disable it), `LATEST_CHAPTER_POLL_ENABLED`/`_COOLDOWN`/`_INTERVAL`/`_BATCH`/`_STAGGER` (background latest-chapter poller; defaults on, `1h`/`10m`/`14`/`20s`). diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md deleted file mode 100644 index 83d17e8..0000000 --- a/backend/CLAUDE.md +++ /dev/null @@ -1,81 +0,0 @@ -Guidance for Claude Code working under `backend/`. See root `CLAUDE.md` for the project-wide architecture diagram, hard constraints, and design system. - -- **Backend** (`backend/`): stdlib `net/http` (handful routes, no framework) + `modernc.org/sqlite` (pure Go, `CGO_ENABLED=0` -> static binary -> distroless/scratch image). Reverse proxy terminates TLS; Go service listens plain `:8080`. - Single binary, split into packages under `backend/internal/`: `store` - (Bookmark type, SQLite persistence, migrations), `latest` (background - poller, site parsers, TLS fetcher), `session` (cookie signing, login - rate limiter), `httpmw` (Auth/Gzip/CORS middleware), `api` (JSON - bookmark handlers), `userscript` (userscript-serving handler), `web` - (browser UI handler + `templates/` + `static/`, `go:embed`-ed). - `backend/main.go` is the composition root — the only place that wires - packages together into `newRouter`. Root-level `*_test.go` hold - integration tests that exercise the full router; unit tests for a - package live beside it under `internal/`. -- **Single-user store.** One `bookmarks` table keyed `:` (`asura`|`demonic`|`comix`|`kagane`). Sync **last-write-wins**. Schema and endpoint list in plan. -- **Endpoints:** `GET /bookmarks`, `PUT /bookmarks/{key}` (upsert; see `updated_at` rule below), `DELETE /bookmarks/{key}`, `GET /healthz` (no auth). -- **Web UI:** same binary serve password-gated browser UI on second - hostname — `GET /` (list, or login page when no session), - `POST /login`, `POST /logout`, `GET /static/*`, htmx fragment endpoints - under `/ui/*`. Templates + assets `go:embed`-ed under - `backend/internal/web/`, so `backend/Dockerfile` must copy the whole - `internal/` tree, not just `*.go`. Sessions stateless - HMAC cookies keyed off `API_TOKEN`; `WEB_PASSWORD` gates them, and when empty, - web routes not registered at all. UI mutations read-modify-write - through `Store.Get` + `Store.Upsert` so `updated_at` rule stays one - place. See `docs/superpowers/specs/2026-07-25-web-ui-design.md`. - **Design-tool caveat:** templates link `/static/style.css` root-absolutely - (correct — served from `/`), but impeccable detector resolves - stylesheet href with `path.resolve(fileDir, href)`, drops directory - on leading `/` and silently skip file. Relative href don't help - either: template's directory isn't its served path. So - `detect.mjs backend/internal/web/templates` reports **false clean** — - always pass `backend/internal/web/static` too. One finding there, - `overused-font` on "Instrument Serif", deliberate identity choice, not debt. -- **Every action that moves series out of list is confirm-gated.** - Archive, finish, remove each open own `.confirm-row` disclosure - (`toggleConfirmRow(key, kind)` in `filter.js`, `kind` ∈ - `archive|finish|remove`); restore fire instantly since it's the reversal. - Remove's row wear ember wash, two reversible ones wear `.calm` grey. - `--ember` stay reserved for new-chapter signal: busy bar and inline - error use `--mute`. -- **Latest-chapter poller:** ticker goroutine in same binary re-check - each bookmarked series' newest published chapter from backend's own - network access, so `latest_chapter` stay fresh when user not - browsing. Second, parallel signal — userscript keep own - `maybeCaptureLatestOnSeriesPage`/`backgroundRefreshLatest` logic unchanged. - Two independent clocks: per-bookmark cooldown (`latest_checked_at` column, - enforced by `Store.DueForLatestCheck`'s WHERE clause) and wake interval. - Row stamped *before* fetch so broken series wait out full - cooldown instead of retrying every tick, and writes go through - `Store.Get` + `Store.Upsert` so new chapter never reorders list. - Fetches use `bogdanfinn/tls-client` with Chrome profile as defence in depth - against fingerprint-based blocking; any failure log and skip. kagane sits - behind a Cloudflare JavaScript challenge the TLS client can't clear, so it is - browser-only: fetched over CDP via `BROWSER_WS_URL`, and simply not polled - when that's unset. See - `docs/superpowers/specs/2026-07-26-server-latest-chapter-polling-design.md`. - Poller's `Store.Get` + `Store.Upsert` not wrapped in transaction, so - userscript `PUT` that commits between the two can get overwritten by - poller's stale re-read — reverting that read progress and, since stored - value now differs, moving `updated_at` and reordering list. Known, - accepted limitation for single-user deployment, not bug to fix. -- **`updated_at` drives list order, so moves only on real reading progress:** server apply its timestamp when row new or `last_chapter_num` changes, else keep stored value — favouriting series or recording newly published chapter must not reorder list. `PUT` therefore returns row **as stored**, clients must adopt that response rather than own payload. See `plans/2026-07-25-bookmark-list-favorites-design.md` §4. -- **Lifecycle buckets:** `status` on each bookmark is `reading` | `archived` | - `finished`, orthogonal to `favorite`. Archived and finished appear only in - own tab — not in All, Updated, Favourites, or recent strip. Poller keeps - checking archived series and skip finished ones. `finished` settable - only from web UI; `PUT /bookmarks/{key}` reject it with 400. - **Empty incoming status means "keep stored one"** — resolved on the - `VALUES` side of `Store.Upsert`, not conflict clause, since - `excluded.*` is post-evaluation row and default applied there would - wipe bucket on every PUT from client that predates column. See - `docs/superpowers/specs/2026-07-27-status-buckets-design.md`. -- **Config via env:** `API_TOKEN`, `ALLOWED_ORIGINS` (comma list), `DB_PATH` - (default `/data/bookmarks.db`), `PORT` (default `8080`), `WEB_PASSWORD` - (gates browser UI; unset disable it), - `LATEST_CHAPTER_POLL_ENABLED`/`_COOLDOWN`/`_INTERVAL`/`_BATCH`/`_STAGGER` - (background latest-chapter poller; defaults on, `1h`/`10m`/`14`/`20s`). - `USERSCRIPT_PATH` (file served at `/u/{token}/manga-bookmark.user.js`, - default `/userscript/manga-bookmark.user.js`, supplied by bindmount). - `BROWSER_WS_URL` (headless-shell CDP endpoint for kagane; unset disables - browser polling and leaves that site to the userscript alone). diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/backend/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/backend/Dockerfile b/backend/Dockerfile index 15afbd4..b40f8b3 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -14,21 +14,17 @@ RUN go mod download COPY *.go ./ COPY internal/ ./internal/ -# Static binary: pure-Go sqlite means CGO_ENABLED=0 -> no libc dependency. +# Static binary: the Postgres driver (jackc/pgx) is pure Go, so CGO_ENABLED=0 +# leaves no libc dependency. # -trimpath + -ldflags strip paths and debug info for a smaller image. RUN CGO_ENABLED=0 GOOS=linux go build -trimpath -ldflags="-s -w" -o /out/server . -# Data dir with the runtime user's ownership so the mounted volume inherits it. -RUN mkdir -p /out/data - # --- runtime stage: distroless static, non-root --- FROM gcr.io/distroless/static:nonroot WORKDIR / COPY --from=build /out/server /server -COPY --from=build --chown=65532:65532 /out/data /data -VOLUME ["/data"] EXPOSE 8080 USER nonroot:nonroot -ENV DB_PATH=/data/bookmarks.db PORT=8080 +ENV PORT=8080 ENTRYPOINT ["/server"] diff --git a/backend/api_test.go b/backend/api_test.go index fafc1fa..e4282c8 100644 --- a/backend/api_test.go +++ b/backend/api_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "bookmarkmanager/backend/internal/pgtest" "bookmarkmanager/backend/internal/store" ) @@ -25,15 +26,21 @@ func testConfig() Config { } } +func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) } + func newTestServer(t *testing.T) http.Handler { t.Helper() - dbPath := filepath.Join(t.TempDir(), "test.db") - s, err := store.Open(dbPath) + return newRouter(newTestStore(t), testConfig()) +} + +func newTestStore(t *testing.T) *store.Store { + t.Helper() + s, err := store.Open(pgtest.URL(t)) if err != nil { t.Fatalf("store.Open: %v", err) } t.Cleanup(func() { s.Close() }) - return newRouter(s, testConfig()) + return s } func auth(req *http.Request) *http.Request { @@ -417,12 +424,7 @@ func TestLoadConfigWebPassword(t *testing.T) { // moved into bookmarkColumns, this test catches it: the PUT would reset the // cooldown and the poller would re-fetch that series on every single tick. func TestPutDoesNotClobberLatestCheckedAt(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "test.db") - s, err := store.Open(dbPath) - if err != nil { - t.Fatalf("store.Open: %v", err) - } - t.Cleanup(func() { s.Close() }) + s := newTestStore(t) srv := newRouter(s, testConfig()) seedForCheck(t, s, "asura:x", "https://asurascans.com/comics/x", 777) @@ -454,12 +456,7 @@ func TestUserscriptServedWithWebUIDisabled(t *testing.T) { t.Fatalf("write script: %v", err) } - dbPath := filepath.Join(t.TempDir(), "nopass.db") - s, err := store.Open(dbPath) - if err != nil { - t.Fatalf("store.Open: %v", err) - } - t.Cleanup(func() { s.Close() }) + s := newTestStore(t) cfg := testConfig() // WebPassword empty cfg.UserscriptPath = path @@ -480,11 +477,7 @@ func TestNovelUserscriptServed(t *testing.T) { t.Fatalf("write script: %v", err) } - s, err := store.Open(filepath.Join(dir, "test.db")) - if err != nil { - t.Fatalf("store.Open: %v", err) - } - t.Cleanup(func() { s.Close() }) + s := newTestStore(t) cfg := testConfig() cfg.NovelUserscriptPath = novelPath diff --git a/backend/go.mod b/backend/go.mod index 43db272..a9848d3 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -7,7 +7,7 @@ require ( github.com/bogdanfinn/tls-client v1.15.1 github.com/chromedp/cdproto v0.0.0-20260714215040-dc233986426f github.com/chromedp/chromedp v0.16.0 - modernc.org/sqlite v1.34.4 + github.com/jackc/pgx/v5 v5.10.0 ) require ( @@ -18,27 +18,19 @@ require ( github.com/bogdanfinn/utls v1.7.7-barnius // indirect github.com/bogdanfinn/websocket v1.5.5-barnius // indirect github.com/chromedp/sysutil v1.1.0 // indirect - github.com/dustin/go-humanize v1.0.1 // indirect github.com/go-json-experiment/json v0.0.0-20260623181947-01eb4420fa68 // indirect github.com/gobwas/httphead v0.1.0 // indirect github.com/gobwas/pool v0.2.1 // indirect github.com/gobwas/ws v1.4.0 // indirect - github.com/google/uuid v1.6.0 // indirect - github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect + github.com/jackc/pgpassfile v1.0.0 // indirect + github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect + github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/klauspost/compress v1.18.2 // indirect - github.com/mattn/go-isatty v0.0.20 // indirect - github.com/ncruces/go-strftime v0.1.9 // indirect github.com/quic-go/qpack v0.6.0 // indirect - github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/tam7t/hpkp v0.0.0-20160821193359-2b70b4024ed5 // indirect golang.org/x/crypto v0.46.0 // indirect golang.org/x/net v0.48.0 // indirect + golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/text v0.32.0 // indirect - modernc.org/gc/v3 v3.0.0-20240107210532-573471604cb6 // indirect - modernc.org/libc v1.55.3 // indirect - modernc.org/mathutil v1.6.0 // indirect - modernc.org/memory v1.8.0 // indirect - modernc.org/strutil v1.2.0 // indirect - modernc.org/token v1.1.0 // indirect ) diff --git a/backend/go.sum b/backend/go.sum index ef779f6..898d9aa 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -20,10 +20,9 @@ github.com/chromedp/chromedp v0.16.0 h1:rOO4deOm4CbZgBCa8mD9g2rDyIoNs0BkgvNrlbp5 github.com/chromedp/chromedp v0.16.0/go.mod h1:rbuGKFT1vMcFcFqKfPIO1GpX/N+2s8onm2qMxZLbU5U= github.com/chromedp/sysutil v1.1.0 h1:PUFNv5EcprjqXZD9nJb9b/c9ibAbxiYo4exNWZyipwM= github.com/chromedp/sysutil v1.1.0/go.mod h1:WiThHUdltqCNKGc4gaU50XgYjwjYIhKWoHGPTUfWTJ8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= -github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/go-json-experiment/json v0.0.0-20260623181947-01eb4420fa68 h1:KZaTBSyshWX3MP5jukJcNSuXDQTO+rNpt0J564dX/eg= github.com/go-json-experiment/json v0.0.0-20260623181947-01eb4420fa68/go.mod h1:tphK2c80bpPhMOI4v6bIc2xWywPfbqi1Z06+RcrMkDg= github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU= @@ -32,28 +31,27 @@ github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og= github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw= github.com/gobwas/ws v1.4.0 h1:CTaoG1tojrh4ucGPcoJFiAQUAsEWekEWvLy7GsVNqGs= github.com/gobwas/ws v1.4.0/go.mod h1:G3gNqMNtPppf5XUz7O4shetPpcZ1VJ7zt18dlUeakrc= -github.com/google/pprof v0.0.0-20240409012703-83162a5b38cd h1:gbpYu9NMq8jhDVbvlGkMFWCjLFlqqEZjEmObmhUy6Vo= -github.com/google/pprof v0.0.0-20240409012703-83162a5b38cd/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw= -github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= -github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= -github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= +github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= +github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= +github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= +github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= +github.com/jackc/pgx/v5 v5.10.0 h1:VhSvgU2jSli8o3AqIEOTJr7rZwAEUVo4E4XhR94Zfr0= +github.com/jackc/pgx/v5 v5.10.0/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= +github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= +github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/klauspost/compress v1.18.2 h1:iiPHWW0YrcFgpBYhsA6D1+fqHssJscY/Tm/y2Uqnapk= github.com/klauspost/compress v1.18.2/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80 h1:6Yzfa6GP0rIo/kULo2bwGEkFvCePZ3qHDDTC3/J9Swo= github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80/go.mod h1:imJHygn/1yfhB7XSJJKlFZKl/J+dCPAknuiaGOshXAs= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/ncruces/go-strftime v0.1.9 h1:bY0MQC28UADQmHmaF5dgpLmImcShSi2kHU9XLdhx/f4= -github.com/ncruces/go-strftime v0.1.9/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde h1:x0TT0RDC7UhAVbbWWBzr41ElhJx5tXPWkIHA2HWPRuw= github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8= github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= -github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= -github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/tam7t/hpkp v0.0.0-20160821193359-2b70b4024ed5 h1:YqAladjX7xpA6BM04leXMWAEjS0mTZ5kUU9KRBriQJc= @@ -64,8 +62,6 @@ go.uber.org/mock v0.5.2 h1:LbtPTcP8A5k9WPXj54PPPbjcI4Y6lhyOZXn+VS7wNko= go.uber.org/mock v0.5.2/go.mod h1:wLlUxC2vVTPTaE3UD51E0BGOAElKrILxhVSDYQLld5o= golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0= -golang.org/x/mod v0.30.0 h1:fDEXFVZ/fmCKProc/yAXXUijritrDzahmwwefnjoPFk= -golang.org/x/mod v0.30.0/go.mod h1:lAsf5O2EvJeSFMiBxXDki7sCgAxEUcZHXoXMKT4GJKc= golang.org/x/net v0.0.0-20211104170005-ce137452f963/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.48.0 h1:zyQRTTrjc33Lhh0fBgT/H3oZq9WuvRR5gPC70xpDiQU= golang.org/x/net v0.48.0/go.mod h1:+ndRgGjkh8FGtu1w1FGbEC31if4VrNVMuKTgcAAnQRY= @@ -81,33 +77,7 @@ golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.39.0 h1:ik4ho21kwuQln40uelmciQPp9SipgNDdrafrYA4TmQQ= -golang.org/x/tools v0.39.0/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -modernc.org/cc/v4 v4.21.4 h1:3Be/Rdo1fpr8GrQ7IVw9OHtplU4gWbb+wNgeoBMmGLQ= -modernc.org/cc/v4 v4.21.4/go.mod h1:HM7VJTZbUCR3rV8EYBi9wxnJ0ZBRiGE5OeGXNA0IsLQ= -modernc.org/ccgo/v4 v4.19.2 h1:lwQZgvboKD0jBwdaeVCTouxhxAyN6iawF3STraAal8Y= -modernc.org/ccgo/v4 v4.19.2/go.mod h1:ysS3mxiMV38XGRTTcgo0DQTeTmAO4oCmJl1nX9VFI3s= -modernc.org/fileutil v1.3.0 h1:gQ5SIzK3H9kdfai/5x41oQiKValumqNTDXMvKo62HvE= -modernc.org/fileutil v1.3.0/go.mod h1:XatxS8fZi3pS8/hKG2GH/ArUogfxjpEKs3Ku3aK4JyQ= -modernc.org/gc/v2 v2.4.1 h1:9cNzOqPyMJBvrUipmynX0ZohMhcxPtMccYgGOJdOiBw= -modernc.org/gc/v2 v2.4.1/go.mod h1:wzN5dK1AzVGoH6XOzc3YZ+ey/jPgYHLuVckd62P0GYU= -modernc.org/gc/v3 v3.0.0-20240107210532-573471604cb6 h1:5D53IMaUuA5InSeMu9eJtlQXS2NxAhyWQvkKEgXZhHI= -modernc.org/gc/v3 v3.0.0-20240107210532-573471604cb6/go.mod h1:Qz0X07sNOR1jWYCrJMEnbW/X55x206Q7Vt4mz6/wHp4= -modernc.org/libc v1.55.3 h1:AzcW1mhlPNrRtjS5sS+eW2ISCgSOLLNyFzRh/V3Qj/U= -modernc.org/libc v1.55.3/go.mod h1:qFXepLhz+JjFThQ4kzwzOjA/y/artDeg+pcYnY+Q83w= -modernc.org/mathutil v1.6.0 h1:fRe9+AmYlaej+64JsEEhoWuAYBkOtQiMEU7n/XgfYi4= -modernc.org/mathutil v1.6.0/go.mod h1:Ui5Q9q1TR2gFm0AQRqQUaBWFLAhQpCwNcuhBOSedWPo= -modernc.org/memory v1.8.0 h1:IqGTL6eFMaDZZhEWwcREgeMXYwmW83LYW8cROZYkg+E= -modernc.org/memory v1.8.0/go.mod h1:XPZ936zp5OMKGWPqbD3JShgd/ZoQ7899TUuQqxY+peU= -modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4= -modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0= -modernc.org/sortutil v1.2.0 h1:jQiD3PfS2REGJNzNCMMaLSp/wdMNieTbKX920Cqdgqc= -modernc.org/sortutil v1.2.0/go.mod h1:TKU2s7kJMf1AE84OoiGppNHJwvB753OYfNl2WRb++Ss= -modernc.org/sqlite v1.34.4 h1:sjdARozcL5KJBvYQvLlZEmctRgW9xqIZc2ncN7PU0P8= -modernc.org/sqlite v1.34.4/go.mod h1:3QQFCG2SEMtc2nv+Wq4cQCH7Hjcg+p/RMlS1XK+zwbk= -modernc.org/strutil v1.2.0 h1:agBi9dp1I+eOnxXeiZawM8F4LawKv4NzGWSaLfyeNZA= -modernc.org/strutil v1.2.0/go.mod h1:/mdcBmfOibveCTBxUl5B5l6W+TTH1FXPLHZE6bTosX0= -modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= -modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM= diff --git a/backend/internal/latest/poller_test.go b/backend/internal/latest/poller_test.go index fdb9985..a9fa3d1 100644 --- a/backend/internal/latest/poller_test.go +++ b/backend/internal/latest/poller_test.go @@ -3,18 +3,21 @@ package latest import ( "context" "errors" - "path/filepath" + "os" "sync" "testing" "time" + "bookmarkmanager/backend/internal/pgtest" "bookmarkmanager/backend/internal/store" ) -// newTestStore opens a fresh SQLite store in a temp dir. +func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) } + +// newTestStore opens a store on a Postgres database of this test's own. func newTestStore(t *testing.T) *store.Store { t.Helper() - s, err := store.Open(filepath.Join(t.TempDir(), "test.db")) + s, err := store.Open(pgtest.URL(t)) if err != nil { t.Fatalf("Open: %v", err) } diff --git a/backend/internal/latest/sites.go b/backend/internal/latest/sites.go index 78d14e0..52ebfa1 100644 --- a/backend/internal/latest/sites.go +++ b/backend/internal/latest/sites.go @@ -5,8 +5,6 @@ import ( "regexp" "strconv" "strings" - - "bookmarkmanager/backend/internal/store" ) // latestChapter is the newest chapter a series page advertises. @@ -22,6 +20,12 @@ type latestChapter struct { // before using the slug to scope anything. var asuraSlugRe = regexp.MustCompile(`/comics/([^/?#]+)`) +// asuraBuildHash matches the trailing "-xxxxxxxx" site-wide build ID Asura +// appends to every series slug. It rotates on each site redeploy, so it is +// never part of a stable series_id. Must stay in sync with stripBuildHash in +// userscript/manga-bookmark.user.js. +var asuraBuildHash = regexp.MustCompile(`-[0-9a-f]{8}$`) + // demonicChapterRe matches the pre-redirect anchors demonic series pages link // through. Both the raw "&" and the HTML-escaped "&" forms occur. var demonicChapterRe = regexp.MustCompile(`chaptered\.php\?manga=\d+&(?:amp;)?chapter=([0-9.]+)`) @@ -74,9 +78,9 @@ func latestChapterFrom(site, seriesURL, body string) (latestChapter, bool) { } // Stored URLs predating a redeploy may carry a stale build hash; // chapter hrefs in the fetched body carry the current one. Strip to - // the stable ID (same rule as migrateAsuraKeys) and make the hash - // optional in the pattern, so scoping survives rotations. - slug := store.AsuraBuildHash.ReplaceAllString(m[1], "") + // the stable ID and make the hash optional in the pattern, so scoping + // survives rotations. + slug := asuraBuildHash.ReplaceAllString(m[1], "") // Compiled per call rather than cached: this runs once per fetch, which // is at most a few times a minute, and the slug varies per series. re = regexp.MustCompile(`/comics/` + regexp.QuoteMeta(slug) + `(?:-[0-9a-f]{8})?/chapter/([0-9.]+)`) diff --git a/backend/internal/pgtest/pgtest.go b/backend/internal/pgtest/pgtest.go new file mode 100644 index 0000000..b4271a7 --- /dev/null +++ b/backend/internal/pgtest/pgtest.go @@ -0,0 +1,120 @@ +// Package pgtest runs the Postgres the test suite needs: one throwaway +// container per test binary, one fresh database per test. Docker is therefore +// a hard prerequisite for `go test ./...`. +// +// Rolled by hand rather than pulled in as a dependency — it is one `docker +// run`, one `docker port` and a ping loop, against a module list that is +// otherwise stdlib plus what the poller genuinely needs. +package pgtest + +import ( + "database/sql" + "fmt" + "os/exec" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" + + _ "github.com/jackc/pgx/v5/stdlib" +) + +const ( + image = "postgres:17-alpine" + readyLimit = 60 * time.Second +) + +var ( + adminURL string + dbSeq atomic.Int64 +) + +// Main starts the container, runs the package's tests and tears the container +// down. Every test package that touches the store calls it from TestMain: +// +// func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) } +func Main(m *testing.M) int { + id, url, err := start() + if err != nil { + fmt.Println("pgtest:", err) + return 1 + } + defer exec.Command("docker", "rm", "-f", id).Run() + + adminURL = url + return m.Run() +} + +// URL creates a database of its own for t and returns a connection URL for it. +// Nothing drops it again: the container goes away wholesale when Main returns. +func URL(t testing.TB) string { + t.Helper() + if adminURL == "" { + t.Fatal("pgtest: no container; this package needs TestMain to call pgtest.Main") + } + // Generated, never derived from the test name, so it needs no quoting and + // cannot collide when tests run in parallel. + name := "test_" + strconv.FormatInt(dbSeq.Add(1), 10) + + admin, err := sql.Open("pgx", adminURL) + if err != nil { + t.Fatalf("pgtest: open admin connection: %v", err) + } + defer admin.Close() + if _, err := admin.Exec(`CREATE DATABASE ` + name); err != nil { + t.Fatalf("pgtest: create database %s: %v", name, err) + } + return strings.Replace(adminURL, "/postgres?", "/"+name+"?", 1) +} + +// start launches the container and waits for it to accept queries, returning +// its id and a connection URL for the default database. +func start() (id, url string, err error) { + out, err := exec.Command("docker", "run", "-d", "--rm", + "-e", "POSTGRES_PASSWORD=pgtest", + "-P", image, + // Durability buys nothing for a database that dies with the test + // binary, and turning it off is most of the container's start-up cost. + "-c", "fsync=off", "-c", "full_page_writes=off", + ).Output() + if err != nil { + return "", "", fmt.Errorf("docker run %s: %w", image, err) + } + id = strings.TrimSpace(string(out)) + + port, err := exec.Command("docker", "port", id, "5432/tcp").Output() + if err != nil { + exec.Command("docker", "rm", "-f", id).Run() + return "", "", fmt.Errorf("docker port: %w", err) + } + // "0.0.0.0:32768" (and possibly a second, IPv6 line); the port is all we want. + first, _, _ := strings.Cut(strings.TrimSpace(string(port)), "\n") + url = fmt.Sprintf("postgres://postgres:pgtest@127.0.0.1:%s/postgres?sslmode=disable", + first[strings.LastIndex(first, ":")+1:]) + + if err := waitReady(url); err != nil { + exec.Command("docker", "rm", "-f", id).Run() + return "", "", err + } + return id, url, nil +} + +func waitReady(url string) error { + db, err := sql.Open("pgx", url) + if err != nil { + return err + } + defer db.Close() + + deadline := time.Now().Add(readyLimit) + for { + if err = db.Ping(); err == nil { + return nil + } + if time.Now().After(deadline) { + return fmt.Errorf("postgres not ready after %s: %w", readyLimit, err) + } + time.Sleep(200 * time.Millisecond) + } +} diff --git a/backend/internal/store/migrations/0001_bookmarks.sql b/backend/internal/store/migrations/0001_bookmarks.sql new file mode 100644 index 0000000..6d817d4 --- /dev/null +++ b/backend/internal/store/migrations/0001_bookmarks.sql @@ -0,0 +1,28 @@ +-- One row per tracked series, keyed ":". +-- +-- Everything is NOT NULL with a default except latest_chapter_num, where NULL +-- is a distinct state: nothing has been captured yet, which is not the same as +-- chapter zero. +-- +-- Timestamps are unix milliseconds as bigint, not timestamptz: the userscripts +-- send Date.now() over the wire and the ordering rule compares them directly. +CREATE TABLE bookmarks ( + key text PRIMARY KEY, + site text NOT NULL, + series_id text NOT NULL, + title text NOT NULL DEFAULT '', + series_url text NOT NULL DEFAULT '', + cover text NOT NULL DEFAULT '', + last_chapter text NOT NULL DEFAULT '', + last_chapter_num double precision NOT NULL DEFAULT 0, + last_chapter_url text NOT NULL DEFAULT '', + favorite boolean NOT NULL DEFAULT false, + latest_chapter text NOT NULL DEFAULT '', + latest_chapter_num double precision, + -- When the server last polled this series, unix ms; 0 means never, and sorts + -- first so a new bookmark is picked up on the next tick with no special case. + latest_checked_at bigint NOT NULL DEFAULT 0, + status text NOT NULL DEFAULT 'reading', + kind text NOT NULL DEFAULT 'manga', + updated_at bigint NOT NULL +); diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index 903e66a..956009a 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -2,13 +2,17 @@ package store import ( "database/sql" + "embed" "errors" "fmt" + "io/fs" + "path" "regexp" + "slices" "strconv" "strings" - _ "modernc.org/sqlite" + _ "github.com/jackc/pgx/v5/stdlib" ) // Bookmark is one tracked series, keyed ":" across both sites. @@ -114,222 +118,119 @@ const ( StatusFinished = "finished" ) -const schema = ` -CREATE TABLE IF NOT EXISTS bookmarks ( - key TEXT PRIMARY KEY, - site TEXT NOT NULL, - series_id TEXT NOT NULL, - title TEXT, - series_url TEXT, - cover TEXT, - last_chapter TEXT, - last_chapter_num REAL, - last_chapter_url TEXT, - favorite INTEGER NOT NULL DEFAULT 0, - latest_chapter TEXT NOT NULL DEFAULT '', - latest_chapter_num REAL, - latest_checked_at INTEGER NOT NULL DEFAULT 0, - status TEXT NOT NULL DEFAULT 'reading', - kind TEXT NOT NULL DEFAULT 'manga', - updated_at INTEGER NOT NULL -);` - -// The columns above that databases created before them will be missing. -// SQLite has no ADD COLUMN IF NOT EXISTS, so each is added only when absent. -var addedColumns = []struct{ name, ddl string }{ - {"favorite", `ALTER TABLE bookmarks ADD COLUMN favorite INTEGER NOT NULL DEFAULT 0`}, - {"latest_chapter", `ALTER TABLE bookmarks ADD COLUMN latest_chapter TEXT NOT NULL DEFAULT ''`}, - {"latest_chapter_num", `ALTER TABLE bookmarks ADD COLUMN latest_chapter_num REAL`}, - // When the server last looked at this series, unix ms; 0 means never, and - // sorts first so a new bookmark is picked up on the next tick with no - // special case. Deliberately NOT in bookmarkColumns — see MarkLatestChecked. - {"latest_checked_at", `ALTER TABLE bookmarks ADD COLUMN latest_checked_at INTEGER NOT NULL DEFAULT 0`}, - // Lifecycle bucket. The DEFAULT backfills every pre-existing row as - // 'reading', so there is no separate migration step. - {"status", `ALTER TABLE bookmarks ADD COLUMN status TEXT NOT NULL DEFAULT 'reading'`}, - // Library bucket. The DEFAULT backfills every pre-existing row as 'manga', - // which is what every row written before novels existed actually is. - {"kind", `ALTER TABLE bookmarks ADD COLUMN kind TEXT NOT NULL DEFAULT 'manga'`}, -} +//go:embed migrations/*.sql +var migrations embed.FS +// bookmarkColumns is the only value ever concatenated into query text. It is a +// compile-time constant; every request value is bound as a parameter. const bookmarkColumns = `key, site, series_id, title, series_url, cover, last_chapter, last_chapter_num, last_chapter_url, favorite, latest_chapter, latest_chapter_num, updated_at, status, kind` -// Store is the SQLite-backed bookmark store. +// Store is the Postgres-backed bookmark store. type Store struct { db *sql.DB } -// OpenStore opens (or creates) the SQLite database at path and applies the schema. -func Open(path string) (*Store, error) { - // busy_timeout guards against SQLITE_BUSY under the reverse proxy's - // concurrent requests; a single writer connection keeps writes serialized. - dsn := path - if !strings.Contains(dsn, "?") { - dsn += "?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)" - } - db, err := sql.Open("sqlite", dsn) +// Open connects to Postgres at url — a libpq connection URL such as +// "postgres://user:pass@host:5432/bookmarks?sslmode=disable" — and brings its +// schema up to date. +func Open(url string) (*Store, error) { + db, err := sql.Open("pgx", url) if err != nil { - return nil, fmt.Errorf("open sqlite %q: %w", path, err) + return nil, fmt.Errorf("open postgres: %w", err) } - db.SetMaxOpenConns(1) - if _, err := db.Exec(schema); err != nil { + if err := migrate(db); err != nil { db.Close() - return nil, fmt.Errorf("apply schema: %w", err) - } - if err := migrateColumns(db); err != nil { - db.Close() - return nil, fmt.Errorf("migrate schema: %w", err) - } - if err := migrateAsuraKeys(db); err != nil { - db.Close() - return nil, fmt.Errorf("migrate asura keys: %w", err) + return nil, fmt.Errorf("migrate: %w", err) } return &Store{db: db}, nil } -// migrateColumns brings a pre-existing bookmarks table up to the current -// schema. Safe to run on every start: columns already present are skipped. -func migrateColumns(db *sql.DB) error { - have, err := existingColumns(db, "bookmarks") +// migrate applies every embedded migration this database has not recorded, in +// filename order, each in its own transaction. Files are named +// "_.sql" and are append-only: editing an applied file changes +// nothing, because schema_migrations is how a database remembers what it ran. +// Runs on every start and is a no-op once current. +func migrate(db *sql.DB) error { + if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations ( + version bigint PRIMARY KEY, + applied_at timestamptz NOT NULL DEFAULT now())`); err != nil { + return fmt.Errorf("create version table: %w", err) + } + + names, err := fs.Glob(migrations, "migrations/*.sql") if err != nil { return err } - for _, c := range addedColumns { - if _, ok := have[c.name]; ok { - continue + slices.Sort(names) + + for _, name := range names { + version, err := strconv.ParseInt(strings.SplitN(path.Base(name), "_", 2)[0], 10, 64) + if err != nil { + return fmt.Errorf("migration %q: filename must start with a version number", name) } - if _, err := db.Exec(c.ddl); err != nil { - return fmt.Errorf("add column %q: %w", c.name, err) + body, err := migrations.ReadFile(name) + if err != nil { + return err + } + if err := applyMigration(db, version, string(body)); err != nil { + return fmt.Errorf("migration %q: %w", name, err) } } return nil } -// AsuraBuildHash matches the trailing "-xxxxxxxx" site-wide build ID Asura -// appends to every series slug. It rotates on each site redeploy, so it -// must not be part of series_id. Must stay in sync with stripBuildHash in -// userscript/manga-bookmark.user.js. -var AsuraBuildHash = regexp.MustCompile(`-[0-9a-f]{8}$`) - -// migrateAsuraKeys rewrites asura bookmarks whose series_id still carries -// the build hash to the stable, hashless ID. Rows keyed with a hash are -// orphaned on every Asura redeploy (old-hash URLs 302 to new-hash ones, so -// detection yields a key that never matches). When two hash-generations of -// one series collide, the row with the newest updated_at wins and the rest -// are deleted. Idempotent: hashless IDs never match the regex. -func migrateAsuraKeys(db *sql.DB) error { - rows, err := db.Query(`SELECT key, series_id, updated_at FROM bookmarks WHERE site = 'asura'`) +// applyMigration runs one migration and records its version in the same +// transaction, so an interrupted start leaves neither half behind. +func applyMigration(db *sql.DB, version int64, body string) error { + tx, err := db.Begin() if err != nil { - return fmt.Errorf("list asura rows: %w", err) - } - type row struct { - key, id string - updated int64 - } - var all []row - for rows.Next() { - var r row - if err := rows.Scan(&r.key, &r.id, &r.updated); err != nil { - rows.Close() - return fmt.Errorf("scan asura row: %w", err) - } - all = append(all, r) - } - if err := rows.Close(); err != nil { return err } + defer tx.Rollback() - groups := map[string][]row{} - for _, r := range all { - stripped := AsuraBuildHash.ReplaceAllString(r.id, "") - groups[stripped] = append(groups[stripped], r) + var applied bool + if err := tx.QueryRow( + `SELECT EXISTS (SELECT 1 FROM schema_migrations WHERE version = $1)`, + version).Scan(&applied); err != nil { + return err } - for stripped, g := range groups { - winner := 0 - for i := range g { - if g[i].updated > g[winner].updated { - winner = i - } - } - // Losers go first: rewriting the winner to the stripped key while a - // pre-existing hashless row still holds it is a primary-key collision. - for i, r := range g { - if i == winner { - continue - } - if _, err := db.Exec(`DELETE FROM bookmarks WHERE key = ?`, r.key); err != nil { - return fmt.Errorf("drop duplicate %q: %w", r.key, err) - } - } - if r := g[winner]; r.id != stripped { - if _, err := db.Exec( - `UPDATE bookmarks SET key = ?, series_id = ? WHERE key = ?`, - "asura:"+stripped, stripped, r.key); err != nil { - return fmt.Errorf("rewrite key %q: %w", r.key, err) - } - } + if applied { + return nil } - return nil + // No parameters, so this goes over the simple protocol and a migration may + // hold more than one statement. + if _, err := tx.Exec(body); err != nil { + return err + } + if _, err := tx.Exec(`INSERT INTO schema_migrations (version) VALUES ($1)`, version); err != nil { + return err + } + return tx.Commit() } -func existingColumns(db *sql.DB, table string) (map[string]struct{}, error) { - rows, err := db.Query(`SELECT name FROM pragma_table_info(?)`, table) - if err != nil { - return nil, fmt.Errorf("read %s columns: %w", table, err) - } - defer rows.Close() - - out := map[string]struct{}{} - for rows.Next() { - var name string - if err := rows.Scan(&name); err != nil { - return nil, fmt.Errorf("scan column name: %w", err) - } - out[name] = struct{}{} - } - return out, rows.Err() -} - -// scanBookmark reads one row in bookmarkColumns order, translating SQLite's -// integer bool and nullable latest_chapter_num into Go types. -// -// The optional columns are read through Null* types because rows predating -// this code (or written by hand) may hold NULL where the app only ever writes -// zero values. Only latest_chapter_num distinguishes the two: everywhere else -// NULL and the zero value mean the same thing to clients. +// scanBookmark reads one row in bookmarkColumns order. Every column is NOT +// NULL except latest_chapter_num, where NULL means "never captured" — a +// distinct state from chapter zero, and the reason for the pointer. func scanBookmark(scan func(...any) error) (Bookmark, error) { var ( - b Bookmark - title, seriesURL, cover sql.NullString - lastChapter, lastChapterURL, latestChapter sql.NullString - status sql.NullString - lastChapterNum, latestChapterNum sql.NullFloat64 - favorite sql.NullInt64 + b Bookmark + latestChapterNum sql.NullFloat64 ) if err := scan( - &b.Key, &b.Site, &b.SeriesID, &title, &seriesURL, &cover, - &lastChapter, &lastChapterNum, &lastChapterURL, - &favorite, &latestChapter, &latestChapterNum, &b.UpdatedAt, &status, &b.Kind, + &b.Key, &b.Site, &b.SeriesID, &b.Title, &b.SeriesURL, &b.Cover, + &b.LastChapter, &b.LastChapterNum, &b.LastChapterURL, + &b.Favorite, &b.LatestChapter, &latestChapterNum, &b.UpdatedAt, &b.Status, &b.Kind, ); err != nil { return Bookmark{}, err } - b.Title = title.String - b.SeriesURL = seriesURL.String - b.Cover = cover.String - b.LastChapter = lastChapter.String - b.LastChapterNum = lastChapterNum.Float64 - b.LastChapterURL = lastChapterURL.String - b.Favorite = favorite.Int64 != 0 - b.LatestChapter = latestChapter.String if latestChapterNum.Valid { b.LatestChapterNum = &latestChapterNum.Float64 } - // A NULL, empty, or unrecognised bucket (e.g. a hand-edited row) would - // leave the row in no list at all, so anything outside the three known - // buckets reads as the default rather than being passed through. - b.Status = status.String + // An unrecognised bucket (a hand-edited row) would leave the row in no list + // at all, so anything outside the three known buckets reads as the default + // rather than being passed through. if b.Status != StatusReading && b.Status != StatusArchived && b.Status != StatusFinished { b.Status = StatusReading } @@ -365,7 +266,7 @@ func (s *Store) List() ([]Bookmark, error) { // the fields they do not touch. func (s *Store) Get(key string) (Bookmark, bool, error) { b, err := scanBookmark(s.db.QueryRow( - `SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = ?`, key).Scan) + `SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = $1`, key).Scan) if errors.Is(err, sql.ErrNoRows) { return Bookmark{}, false, nil } @@ -395,9 +296,10 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) { latestNum = *b.LatestChapterNum } - // IS NOT is SQLite's null-safe comparison. Within DO UPDATE, a bare column - // is the stored row and excluded.* is the incoming one; a brand-new key - // never reaches this clause, so it keeps the fresh timestamp from VALUES. + // IS DISTINCT FROM is Postgres's null-safe comparison, and it is what + // implements the ordering rule. Within DO UPDATE, a bare column is the + // stored row and excluded.* is the incoming one; a brand-new key never + // reaches this clause, so it keeps the fresh timestamp from VALUES. // // The status and kind columns resolve on the VALUES side, not in the // conflict clause: excluded.* is the row *after* these expressions are @@ -408,12 +310,16 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) { // stored", and only a brand-new row falls through to the literal // default. The subquery runs inside this transaction, so it sees the // row this statement is about to conflict with. + // + // The ::text casts are load-bearing: inside COALESCE/NULLIF there is no + // target column to infer the parameter type from, and Postgres rejects the + // statement rather than guessing. if _, err := tx.Exec(` INSERT INTO bookmarks (`+bookmarkColumns+`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, - COALESCE(NULLIF(?, ''), (SELECT status FROM bookmarks WHERE key = ?), 'reading'), - COALESCE(NULLIF(?, ''), (SELECT kind FROM bookmarks WHERE key = ?), 'manga')) - ON CONFLICT(key) DO UPDATE SET + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, + COALESCE(NULLIF($14::text, ''), (SELECT status FROM bookmarks WHERE key = $1), 'reading'), + COALESCE(NULLIF($15::text, ''), (SELECT kind FROM bookmarks WHERE key = $1), 'manga')) + ON CONFLICT (key) DO UPDATE SET site=excluded.site, series_id=excluded.series_id, title=excluded.title, series_url=excluded.series_url, cover=excluded.cover, last_chapter=excluded.last_chapter, last_chapter_num=excluded.last_chapter_num, @@ -424,20 +330,19 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) { status=excluded.status, kind=excluded.kind, updated_at=CASE - WHEN bookmarks.last_chapter_num IS NOT excluded.last_chapter_num + WHEN bookmarks.last_chapter_num IS DISTINCT FROM excluded.last_chapter_num THEN excluded.updated_at ELSE bookmarks.updated_at END`, b.Key, b.Site, b.SeriesID, b.Title, b.SeriesURL, b.Cover, b.LastChapter, b.LastChapterNum, b.LastChapterURL, b.Favorite, b.LatestChapter, latestNum, b.UpdatedAt, - b.Status, b.Key, - b.Kind, b.Key); err != nil { + b.Status, b.Kind); err != nil { return Bookmark{}, fmt.Errorf("upsert %q: %w", b.Key, err) } stored, err := scanBookmark(tx.QueryRow( - `SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = ?`, b.Key).Scan) + `SELECT `+bookmarkColumns+` FROM bookmarks WHERE key = $1`, b.Key).Scan) if err != nil { return Bookmark{}, fmt.Errorf("read back %q: %w", b.Key, err) } @@ -449,7 +354,7 @@ func (s *Store) Upsert(b Bookmark) (Bookmark, error) { // Delete removes a bookmark by key. Deleting a missing key is not an error. func (s *Store) Delete(key string) error { - if _, err := s.db.Exec(`DELETE FROM bookmarks WHERE key = ?`, key); err != nil { + if _, err := s.db.Exec(`DELETE FROM bookmarks WHERE key = $1`, key); err != nil { return fmt.Errorf("delete %q: %w", key, err) } return nil @@ -472,11 +377,11 @@ func (s *Store) Delete(key string) error { func (s *Store) DueForLatestCheck(cutoffMs int64, limit int) ([]Bookmark, error) { rows, err := s.db.Query(`SELECT `+bookmarkColumns+` FROM bookmarks - WHERE series_url IS NOT NULL AND series_url <> '' - AND status IS NOT 'finished' - AND latest_checked_at <= ? + WHERE series_url <> '' + AND status IS DISTINCT FROM 'finished' + AND latest_checked_at <= $1 ORDER BY latest_checked_at ASC - LIMIT ?`, cutoffMs, limit) + LIMIT $2`, cutoffMs, limit) if err != nil { return nil, fmt.Errorf("query due bookmarks: %w", err) } @@ -505,7 +410,7 @@ func (s *Store) DueForLatestCheck(cutoffMs int64, limit int) ([]Bookmark, error) // long as the user kept reading it. func (s *Store) MarkLatestChecked(key string, ts int64) error { if _, err := s.db.Exec( - `UPDATE bookmarks SET latest_checked_at = ? WHERE key = ?`, ts, key); err != nil { + `UPDATE bookmarks SET latest_checked_at = $1 WHERE key = $2`, ts, key); err != nil { return fmt.Errorf("mark checked %q: %w", key, err) } return nil @@ -518,7 +423,7 @@ func (s *Store) MarkLatestChecked(key string, ts int64) error { func (s *Store) LatestCheckedAt(key string) (int64, error) { var ts int64 if err := s.db.QueryRow( - `SELECT latest_checked_at FROM bookmarks WHERE key = ?`, key).Scan(&ts); err != nil { + `SELECT latest_checked_at FROM bookmarks WHERE key = $1`, key).Scan(&ts); err != nil { return 0, fmt.Errorf("latest checked at %q: %w", key, err) } return ts, nil diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 7a29188..c444f02 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -1,80 +1,54 @@ package store import ( - "database/sql" - "path/filepath" + "os" "testing" "time" + + "bookmarkmanager/backend/internal/pgtest" ) +func TestMain(m *testing.M) { os.Exit(pgtest.Main(m)) } + func newTestStore(t *testing.T) *Store { t.Helper() - store, err := Open(filepath.Join(t.TempDir(), "test.db")) + store, err := Open(pgtest.URL(t)) if err != nil { - t.Fatalf("OpenStore: %v", err) + t.Fatalf("Open: %v", err) } t.Cleanup(func() { store.Close() }) return store } -func TestOpenStoreMigratesLegacySchema(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "legacy.db") - - legacy, err := sql.Open("sqlite", dbPath) +// The migration runner runs on every start, so a second Open against a +// database it already built must be a no-op rather than a duplicate-table +// error, and must leave the rows alone. +func TestOpenIsIdempotent(t *testing.T) { + url := pgtest.URL(t) + first, err := Open(url) if err != nil { - t.Fatalf("open legacy db: %v", err) + t.Fatalf("Open: %v", err) } - if _, err := legacy.Exec(` - CREATE TABLE bookmarks ( - key TEXT PRIMARY KEY, - site TEXT NOT NULL, - series_id TEXT NOT NULL, - title TEXT, - series_url TEXT, - cover TEXT, - last_chapter TEXT, - last_chapter_num REAL, - last_chapter_url TEXT, - updated_at INTEGER NOT NULL - )`); err != nil { - t.Fatalf("create legacy schema: %v", err) - } - if _, err := legacy.Exec(` - INSERT INTO bookmarks (key, site, series_id, title, last_chapter, last_chapter_num, updated_at) - VALUES ('asura:legacy', 'asura', 'legacy', 'Legacy Series', 'Chapter 7', 7, 123)`); err != nil { - t.Fatalf("seed legacy row: %v", err) - } - if err := legacy.Close(); err != nil { - t.Fatalf("close legacy db: %v", err) + if _, err := first.Upsert(Bookmark{ + Key: "asura:solo", Site: "asura", SeriesID: "solo", UpdatedAt: 1000, + }); err != nil { + t.Fatalf("seed: %v", err) } + first.Close() - store, err := Open(dbPath) + second, err := Open(url) if err != nil { - t.Fatalf("OpenStore on legacy db: %v", err) + t.Fatalf("reopen: %v", err) } - t.Cleanup(func() { store.Close() }) + t.Cleanup(func() { second.Close() }) - list, err := store.List() + list, err := second.List() if err != nil { t.Fatalf("List: %v", err) } - if len(list) != 1 || list[0].Key != "asura:legacy" { - t.Fatalf("legacy row lost: %+v", list) + if len(list) != 1 || list[0].Key != "asura:solo" { + t.Fatalf("rows after reopen = %+v, want only the seeded one", list) } - got := list[0] - if got.Title != "Legacy Series" || got.LastChapterNum != 7 || got.UpdatedAt != 123 { - t.Fatalf("legacy data mangled: %+v", got) - } - if got.Favorite || got.LatestChapter != "" || got.LatestChapterNum != nil { - t.Fatalf("new columns should default empty, got %+v", got) - } - - // Reopening an already-migrated database must be a no-op, not an error. - store2, err := Open(dbPath) - if err != nil { - t.Fatalf("OpenStore is not idempotent: %v", err) - } - store2.Close() } func TestStoreGet(t *testing.T) { @@ -155,7 +129,7 @@ func readLatestCheckedAt(t *testing.T, s *Store, key string) int64 { t.Helper() var ts int64 if err := s.db.QueryRow( - `SELECT latest_checked_at FROM bookmarks WHERE key = ?`, key).Scan(&ts); err != nil { + `SELECT latest_checked_at FROM bookmarks WHERE key = $1`, key).Scan(&ts); err != nil { t.Fatalf("read latest_checked_at %q: %v", key, err) } return ts @@ -264,53 +238,6 @@ func TestUpsertPreservesLatestCheckedAt(t *testing.T) { } } -// migrateColumns must be able to bring a database created before this column up -// to date, not just create it fresh. -func TestMigrateAddsLatestCheckedAt(t *testing.T) { - path := filepath.Join(t.TempDir(), "old.db") - - old, err := sql.Open("sqlite", path) - if err != nil { - t.Fatalf("open: %v", err) - } - // A pre-latest_checked_at table, matching the schema as it shipped before. - if _, err := old.Exec(`CREATE TABLE bookmarks ( - key TEXT PRIMARY KEY, site TEXT NOT NULL, series_id TEXT NOT NULL, - title TEXT, series_url TEXT, cover TEXT, - last_chapter TEXT, last_chapter_num REAL, last_chapter_url TEXT, - favorite INTEGER NOT NULL DEFAULT 0, - latest_chapter TEXT NOT NULL DEFAULT '', latest_chapter_num REAL, - updated_at INTEGER NOT NULL)`); err != nil { - t.Fatalf("create old table: %v", err) - } - if _, err := old.Exec( - `INSERT INTO bookmarks (key, site, series_id, series_url, updated_at) - VALUES ('asura:x', 'asura', 'x', 'https://asurascans.com/comics/x', 5)`); err != nil { - t.Fatalf("seed old row: %v", err) - } - if err := old.Close(); err != nil { - t.Fatalf("close: %v", err) - } - - s, err := Open(path) - if err != nil { - t.Fatalf("OpenStore on pre-existing db: %v", err) - } - t.Cleanup(func() { s.Close() }) - - // The migrated row must default to 0 (never checked) and so be due. - if got := readLatestCheckedAt(t, s, "asura:x"); got != 0 { - t.Fatalf("migrated latest_checked_at = %d, want 0", got) - } - due, err := s.DueForLatestCheck(1000, 10) - if err != nil { - t.Fatalf("DueForLatestCheck: %v", err) - } - if len(due) != 1 { - t.Fatalf("got %d due rows after migration, want 1", len(due)) - } -} - func TestUpsertDefaultsStatusToReading(t *testing.T) { store := newTestStore(t) stored, err := store.Upsert(Bookmark{ @@ -425,41 +352,6 @@ func TestUpsertStatusChangeKeepsUpdatedAt(t *testing.T) { } } -// A database written before the column existed must gain it, with every -// pre-existing row landing in the reading bucket. -func TestMigrationAddsStatusToLegacyDatabase(t *testing.T) { - path := filepath.Join(t.TempDir(), "legacy.db") - db, err := sql.Open("sqlite", path) - if err != nil { - t.Fatalf("open: %v", err) - } - if _, err := db.Exec(` - CREATE TABLE bookmarks ( - key TEXT PRIMARY KEY, site TEXT NOT NULL, series_id TEXT NOT NULL, - title TEXT, series_url TEXT, cover TEXT, - last_chapter TEXT, last_chapter_num REAL, last_chapter_url TEXT, - updated_at INTEGER NOT NULL); - INSERT INTO bookmarks (key, site, series_id, updated_at) - VALUES ('asura:old', 'asura', 'old', 1)`); err != nil { - t.Fatalf("seed legacy: %v", err) - } - db.Close() - - store, err := Open(path) - if err != nil { - t.Fatalf("OpenStore: %v", err) - } - defer store.Close() - - b, ok, err := store.Get("asura:old") - if err != nil || !ok { - t.Fatalf("Get: ok=%v err=%v", ok, err) - } - if b.Status != StatusReading { - t.Fatalf("Status = %q, want %q", b.Status, StatusReading) - } -} - // Archiving is the reason to keep polling — the point is to come back to a // series that has moved on. A finished series has nothing left to publish. func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) { @@ -494,135 +386,6 @@ func TestDueForLatestCheckSkipsFinishedKeepsArchived(t *testing.T) { } } -// Asura slugs used to include the site build hash; rows keyed with it must -// be rewritten to the stable ID on open, merging hash-generations of the -// same series into the newest row. -func TestOpenStoreMigratesAsuraBuildHashKeys(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "hash.db") - - store, err := Open(dbPath) - if err != nil { - t.Fatalf("open: %v", err) - } - seed := []Bookmark{ - {Key: "asura:swordmasters-youngest-son-f886a8af", Site: "asura", - SeriesID: "swordmasters-youngest-son-f886a8af", Title: "Old gen", - LastChapterNum: 50, UpdatedAt: 100}, - {Key: "asura:swordmasters-youngest-son-059befe1", Site: "asura", - SeriesID: "swordmasters-youngest-son-059befe1", Title: "Re-bookmarked", - LastChapterNum: 60, UpdatedAt: 200}, - {Key: "asura:overgeared-059befe1", Site: "asura", - SeriesID: "overgeared-059befe1", Title: "Single gen", UpdatedAt: 150}, - // Hash-like suffix on another site must be left alone. - {Key: "demonic:x-deadbeef", Site: "demonic", - SeriesID: "x-deadbeef", Title: "Not asura", UpdatedAt: 300}, - } - for _, b := range seed { - if _, err := store.Upsert(b); err != nil { - t.Fatalf("seed %s: %v", b.Key, err) - } - } - if err := store.Close(); err != nil { - t.Fatalf("close: %v", err) - } - - reopened, err := Open(dbPath) - if err != nil { - t.Fatalf("reopen: %v", err) - } - t.Cleanup(func() { reopened.Close() }) - - list, err := reopened.List() - if err != nil { - t.Fatalf("List: %v", err) - } - byKey := map[string]Bookmark{} - for _, b := range list { - byKey[b.Key] = b - } - if len(list) != 3 { - t.Fatalf("want 3 rows after merge, got %d: %+v", len(list), list) - } - merged, ok := byKey["asura:swordmasters-youngest-son"] - if !ok { - t.Fatalf("merged key missing: %+v", byKey) - } - // Newest row wins the merge. - if merged.Title != "Re-bookmarked" || merged.LastChapterNum != 60 || merged.UpdatedAt != 200 { - t.Fatalf("merge kept wrong row: %+v", merged) - } - if merged.SeriesID != "swordmasters-youngest-son" { - t.Fatalf("series_id not stripped: %q", merged.SeriesID) - } - if _, ok := byKey["asura:overgeared-059befe1"]; ok { - t.Fatal("single-generation hashed key not rewritten") - } - if _, ok := byKey["asura:overgeared"]; !ok { - t.Fatal("single-generation row missing under stripped key") - } - if _, ok := byKey["demonic:x-deadbeef"]; !ok { - t.Fatal("non-asura row touched") - } - - // Idempotent: a third open changes nothing. - third, err := Open(dbPath) - if err != nil { - t.Fatalf("third open: %v", err) - } - third.Close() -} - -// A hashed row and a pre-existing hashless row of the same series collide on -// the stripped key. The winner rewrite must happen only after the loser is -// gone, or the UPDATE hits a primary-key collision and OpenStore fails. -func TestOpenStoreMigratesAsuraHashlessCollision(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "collision.db") - - store, err := Open(dbPath) - if err != nil { - t.Fatalf("open: %v", err) - } - seed := []Bookmark{ - {Key: "asura:overgeared", Site: "asura", SeriesID: "overgeared", - Title: "Hashless", LastChapterNum: 10, UpdatedAt: 100}, - {Key: "asura:overgeared-059befe1", Site: "asura", - SeriesID: "overgeared-059befe1", Title: "Hashed newer", - LastChapterNum: 20, UpdatedAt: 200}, - } - for _, b := range seed { - if _, err := store.Upsert(b); err != nil { - t.Fatalf("seed %s: %v", b.Key, err) - } - } - if err := store.Close(); err != nil { - t.Fatalf("close: %v", err) - } - - reopened, err := Open(dbPath) - if err != nil { - t.Fatalf("reopen: %v", err) - } - t.Cleanup(func() { reopened.Close() }) - - list, err := reopened.List() - if err != nil { - t.Fatalf("List: %v", err) - } - if len(list) != 1 { - t.Fatalf("want 1 row after merge, got %d: %+v", len(list), list) - } - merged := list[0] - if merged.Key != "asura:overgeared" { - t.Fatalf("merged key = %q, want asura:overgeared", merged.Key) - } - if merged.SeriesID != "overgeared" { - t.Fatalf("series_id = %q, want overgeared", merged.SeriesID) - } - if merged.Title != "Hashed newer" || merged.LastChapterNum != 20 || merged.UpdatedAt != 200 { - t.Fatalf("merge kept wrong row: %+v", merged) - } -} - func TestDisplayChapter(t *testing.T) { cases := []struct { name string @@ -713,51 +476,3 @@ func TestUpsertEmptyKindKeepsStoredValue(t *testing.T) { t.Fatalf("LastChapterNum = %v, want 11 — progress in the same request must still land", got.LastChapterNum) } } - -// A database created before this column exists must gain it, backfilled as -// manga, without losing anything. -func TestLegacyDatabaseGainsKindAsManga(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "legacy.db") - - legacy, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatalf("open legacy db: %v", err) - } - if _, err := legacy.Exec(` - CREATE TABLE bookmarks ( - key TEXT PRIMARY KEY, - site TEXT NOT NULL, - series_id TEXT NOT NULL, - title TEXT, - series_url TEXT, - cover TEXT, - last_chapter TEXT, - last_chapter_num REAL, - last_chapter_url TEXT, - updated_at INTEGER NOT NULL - )`); err != nil { - t.Fatalf("create legacy schema: %v", err) - } - if _, err := legacy.Exec(` - INSERT INTO bookmarks (key, site, series_id, title, updated_at) - VALUES ('asura:legacy', 'asura', 'legacy', 'Legacy Series', 123)`); err != nil { - t.Fatalf("seed legacy row: %v", err) - } - if err := legacy.Close(); err != nil { - t.Fatalf("close legacy db: %v", err) - } - - store, err := Open(dbPath) - if err != nil { - t.Fatalf("Open on legacy db: %v", err) - } - t.Cleanup(func() { store.Close() }) - - list, err := store.List() - if err != nil { - t.Fatalf("List: %v", err) - } - if len(list) != 1 || list[0].Kind != KindManga { - t.Fatalf("legacy row should backfill as manga, got %+v", list) - } -} diff --git a/backend/main.go b/backend/main.go index 275f5b8..b27ee15 100644 --- a/backend/main.go +++ b/backend/main.go @@ -24,8 +24,10 @@ import ( type Config struct { Token string AllowedOrigins []string - DBPath string - Port string + // DatabaseURL is the Postgres connection URL; required, no default, + // because a wrong guess would silently start on an empty database. + DatabaseURL string + Port string // WebPassword gates the browser UI. Empty disables the web routes entirely. WebPassword string // UserscriptPath is the file served at /u/{token}/manga-bookmark.user.js. @@ -143,7 +145,7 @@ func loadLatestPoll() LatestPoll { func loadConfig() Config { c := Config{ Token: os.Getenv("API_TOKEN"), - DBPath: envOr("DB_PATH", "/data/bookmarks.db"), + DatabaseURL: os.Getenv("DATABASE_URL"), Port: envOr("PORT", "8080"), WebPassword: os.Getenv("WEB_PASSWORD"), UserscriptPath: envOr("USERSCRIPT_PATH", "/userscript/manga-bookmark.user.js"), @@ -215,8 +217,11 @@ func main() { if cfg.Token == "" { log.Fatal("API_TOKEN is required") } + if cfg.DatabaseURL == "" { + log.Fatal("DATABASE_URL is required") + } - s, err := store.Open(cfg.DBPath) + s, err := store.Open(cfg.DatabaseURL) if err != nil { log.Fatalf("open store: %v", err) } @@ -236,7 +241,8 @@ func main() { } go func() { - log.Printf("listening on :%s (db=%s, origins=%v)", cfg.Port, cfg.DBPath, cfg.AllowedOrigins) + // The connection URL carries a password, so it stays out of the log. + log.Printf("listening on :%s (origins=%v)", cfg.Port, cfg.AllowedOrigins) if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { log.Fatalf("serve: %v", err) } diff --git a/backend/web_test.go b/backend/web_test.go index 0dd662c..4c3f514 100644 --- a/backend/web_test.go +++ b/backend/web_test.go @@ -5,7 +5,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "path/filepath" "strconv" "strings" "testing" @@ -28,11 +27,7 @@ func webConfig() Config { // can seed rows and assert on what the handlers wrote back. func newWebTestServer(t *testing.T, cfg Config) (http.Handler, *store.Store) { t.Helper() - st, err := store.Open(filepath.Join(t.TempDir(), "test.db")) - if err != nil { - t.Fatalf("store.Open: %v", err) - } - t.Cleanup(func() { st.Close() }) + st := newTestStore(t) return newRouter(st, cfg), st } diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 4851023..031e574 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -23,13 +23,18 @@ services: # isn't an IP or "localhost". BROWSER_WS_URL: ${BROWSER_WS_URL:-ws://172.28.0.10:9222} depends_on: - - headless-shell - # `networks:` here replaces the base file's list entirely, so both must be - # named: `proxy` for Traefik routing, `browser` (defined in the base file) - # to keep reaching headless-shell without putting it on `proxy` too. + headless-shell: + condition: service_started + postgres: + condition: service_healthy + # `networks:` here replaces the base file's list entirely, so all three must + # be named: `proxy` for Traefik routing, and `browser` / `db` (defined in + # the base file) to keep reaching headless-shell and Postgres without + # putting either on `proxy`. networks: - proxy - browser + - db labels: - "traefik.enable=true" - "traefik.docker.network=${PROXY_NETWORK:-proxy}" diff --git a/docker-compose.yml b/docker-compose.yml index bf3f0fb..7247598 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -16,7 +16,9 @@ services: # API_TOKEN is required — compose refuses to start without it. API_TOKEN: ${API_TOKEN:?set API_TOKEN in .env} ALLOWED_ORIGINS: ${ALLOWED_ORIGINS:-https://asuracomic.net,https://asurascans.com,https://demonicscans.org,https://comix.to,https://kagane.to,https://novelfull.com,https://lightnovelworld.net} - DB_PATH: /data/bookmarks.db + # The bookmarks database. Host is the compose service name; the password + # comes from .env so it is never committed. + DATABASE_URL: ${DATABASE_URL:-postgres://bookmarks:${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in .env}@postgres:5432/bookmarks?sslmode=disable} PORT: "8080" # Gates the browser UI. Unset means the web routes are not served at all. WEB_PASSWORD: ${WEB_PASSWORD:-} @@ -42,9 +44,13 @@ services: # this URL survives container recreation. BROWSER_WS_URL: ${BROWSER_WS_URL:-ws://172.28.0.10:9222} depends_on: - - headless-shell + headless-shell: + condition: service_started + # 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. + postgres: + condition: service_healthy volumes: - - bookmarks-data:/data # The userscript is served from here, read fresh on every request. Editing # the file in this checkout takes effect on the next Violentmonkey poll — # no rebuild, no restart. `git pull` restores the committed version, which @@ -56,6 +62,26 @@ services: - "127.0.0.1:8080:8080" networks: - browser + - db + + postgres: + image: postgres:17-alpine + restart: unless-stopped + environment: + POSTGRES_DB: bookmarks + POSTGRES_USER: bookmarks + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in .env} + healthcheck: + test: ["CMD-SHELL", "pg_isready -U bookmarks -d bookmarks"] + interval: 5s + timeout: 3s + retries: 10 + volumes: + - postgres-data:/var/lib/postgresql/data + # Deliberately no `ports:` — only bookmark-api, over the `db` network, + # reaches it. Use `docker compose exec postgres psql` for a shell. + networks: + - db headless-shell: image: chromedp/headless-shell:stable @@ -85,7 +111,11 @@ services: ipv4_address: 172.28.0.10 volumes: - bookmarks-data: + postgres-data: + # The pre-Postgres SQLite volume (bookmarks-data) is deliberately no longer + # declared here: undeclared means `docker compose down -v` cannot take it + # with the rest, so the old database survives the cutover until someone + # removes it by hand. networks: # Not `internal: true`: headless Chrome still needs outbound access to reach @@ -95,3 +125,7 @@ networks: ipam: config: - subnet: 172.28.0.0/24 + # Postgres needs no egress and nothing outside bookmark-api needs to reach + # it, so this one really can be cut off from the outside world. + db: + internal: true diff --git a/docs/adr/0001-postgresql-over-sqlite.md b/docs/adr/0001-postgresql-over-sqlite.md new file mode 100644 index 0000000..031c145 --- /dev/null +++ b/docs/adr/0001-postgresql-over-sqlite.md @@ -0,0 +1,40 @@ +# Postgres replaces SQLite as the primary datastore + +Status: accepted + +The project is moving from a single-reader tracker to a service published to a community, +so we replaced `modernc.org/sqlite` with Postgres (`jackc/pgx/v5`, still pure Go, so +`CGO_ENABLED=0` and the distroless image are unaffected). The deciding reason is future +supportability — managed hosting, a datastore that survives the app outgrowing one box — +**not** concurrency, which was measured and found to be a non-issue. + +## Considered options + +**Stay on SQLite.** Benchmarked against the real store at 1,500 rows (≈50 readers × 30 +series): ~9,700 upserts/sec single-writer, plateauing at ~780/sec under 8–50 concurrent +writers, with `List()` holding 90–103 calls/sec under continuous write load. Projected +real load at 50 readers is ~0.02 writes/sec — roughly four and a half orders of magnitude +of headroom. `SetMaxOpenConns(1)` serialises writes but was shown not to starve reads; +an apparent read collapse traced to row count and per-row scanning, not lock contention. +SQLite would have worked. It was rejected for where the project is going, not for what +it does today. + +**Postgres.** Chosen. Migrating is cheapest now — 29 rows in one table — and gets +materially harder once there are live readers and a multi-tenant schema. + +## Consequences + +- Every statement in `internal/store` is rewritten: `?` → `$N`, `IS NOT` → + `IS DISTINCT FROM` (this one is load-bearing; it implements the `updated_at` + ordering rule), `pragma_table_info` → `information_schema.columns`, + `INTEGER`/`REAL` → `bigint`/`double precision`, `favorite` int-as-bool → `boolean`. +- ~75 tests currently get a free isolated database from `t.TempDir()`. They now need a + live server, which makes Docker a hard prerequisite for `go test ./...`. This is the + permanent cost of the decision and the main reason it was close. +- Backups get worse, not better: `VACUUM INTO` produced one self-contained file; + restoring now means `pg_dump`/`pg_restore`, a role, and a password. +- A second stateful container joins the VPS alongside the existing headless-shell. +- **Postgres does not address the real scaling limit.** At batch 14 per 10-minute tick + the poller checks at most 84 series/hour; 50 readers × 30 series is 1,500 bookmarks, + an 18-hour sweep against a configured 1-hour cooldown. That ceiling is an outbound + fetch budget and is fixed by deduplicating polls per Series, not by the datastore. diff --git a/docs/adr/0002-discord-oauth-no-passwords-no-email.md b/docs/adr/0002-discord-oauth-no-passwords-no-email.md new file mode 100644 index 0000000..a1a4930 --- /dev/null +++ b/docs/adr/0002-discord-oauth-no-passwords-no-email.md @@ -0,0 +1,44 @@ +# Identity comes from Discord OAuth; we store no passwords and send no email + +Status: accepted + +The service is being published to a community that already lives on Discord, and we have +no transactional email infrastructure. Rather than build email verification and password +reset to get accounts, Readers sign in with Discord OAuth2 (authorization code grant, +`identify` + `guilds.members.read`), and guild membership replaces both the invite gate +and the email-verification step. No password is ever stored and no mail is ever sent. + +## Considered options + +**Email + password with invite codes, no verification.** Viable and dependency-free: +an invite code proves community membership, which is what email verification was +standing in for anyway. Rejected because it still requires password hashing, a manual +admin-driven reset path, and a credential store — all of which Discord removes. + +**Email + password with a transactional provider** (Resend, Brevo). Rejected as +premature: it builds verification and self-serve reset before anyone has asked for them, +and adds deliverability as an operational concern. + +**Discord OAuth.** Chosen. It is less code than either alternative — no hashing, no +reset flow, no invite table — and the authorization question ("is this person in my +community?") is answered by the same call that answers the authentication question. + +## Consequences + +- **Availability is now coupled to Discord.** If Discord's OAuth endpoint is down, + nobody can start a new session. Existing sessions are unaffected, which bounds the + blast radius. +- **Identity is a Discord snowflake.** Migrating off Discord later means re-identifying + every Reader, because we hold no other credential for them. This is the lock-in the + decision buys, and it is the reason this ADR exists. +- **`guilds.members.read` is checked at login, not continuously.** Someone who leaves + the guild keeps their session until it expires. Acceptable; revocation is a session + delete, not an architectural change. +- **The userscripts cannot use OAuth.** They run in an isolated world on third-party + pages with no redirect surface, so they keep a bearer token — now issued per Reader by + the backend rather than a single shared `API_TOKEN` literal. OAuth gates the web UI; + the web UI is where a Reader obtains their personal userscript. +- `WEB_PASSWORD` disappears, and with it the session HMAC key derivation + (`sha256(API_TOKEN | WEB_PASSWORD | …)`), which needs a replacement secret. +- Seeding the first Reader during migration requires knowing the owner's Discord user + ID up front — a stable snowflake, copied from the Discord client. diff --git a/docs/adr/0003-series-shared-and-poll-owned.md b/docs/adr/0003-series-shared-and-poll-owned.md new file mode 100644 index 0000000..91c52cb --- /dev/null +++ b/docs/adr/0003-series-shared-and-poll-owned.md @@ -0,0 +1,44 @@ +# Series is a shared entity, and only the Poll may update it + +Status: accepted + +Facts about a Series that are true regardless of who is reading — title, cover, canonical +URL, Latest Chapter — moved off the Bookmark onto a shared `series` row keyed +`(site, series_id)`. A Bookmark now holds only what differs between Readers: Progress, +Favourite, Lifecycle bucket. Fifty Readers tracking one Series produce fifty Bookmarks +and one Series, so the Series is polled once rather than fifty times. + +## Why + +The poller checks at most 84 series/hour (batch 14 per 10-minute tick). With ~50 Readers +holding ~30 Series each, polling per Bookmark means a 1,500-item sweep — roughly 18 hours +against a configured 1-hour cooldown, quietly breaking the New Chapter signal that is the +product's reason to exist. Deduplicating to distinct Series cuts the sweep several-fold, +and because the Series row now knows how many Readers hold it, the poll queue is ordered +`reader_count DESC, latest_checked_at ASC` — popular Series stay fresh and the long tail +absorbs the shortfall. That ordering is only expressible because the split happened. + +Raising throughput instead was rejected: sweeping 400 Series hourly needs the stagger +cut from 20s to ~9s, doubling request rate against sites that already bot-score the +single VPS IP. + +## Only the Poll writes Series fields + +A client may supply `title`, `cover` and `series_url` only when creating a Series nobody +has bookmarked yet. After that, client-supplied values are ignored; only the backend's +own fetch updates them. + +This is a security boundary, not tidiness. Those values are scraped from third-party +pages, which `AGENTS.md` requires be treated as attacker-controlled. Before the split, a +hostile or compromised site could corrupt exactly one Reader's row. After it, the same +write lands on a row every Reader sees — one Reader's browser becomes a write path into +everyone else's UI, and a cover URL can point anywhere. The backend's own fetch is the +higher-trust source: its network, its parser, no third-party JavaScript in the path. + +## Consequences + +- `Store.Upsert` decomposes one incoming flat body across two tables and enforces the + ownership rule at that seam. +- The `updated_at` ordering rule stays on the Bookmark, where Progress lives. Unchanged. +- Per-Reader title overrides are deliberately not supported; they would reintroduce the + duplication this removes. diff --git a/docs/adr/0004-wire-format-does-not-mirror-the-schema.md b/docs/adr/0004-wire-format-does-not-mirror-the-schema.md new file mode 100644 index 0000000..ee4084d --- /dev/null +++ b/docs/adr/0004-wire-format-does-not-mirror-the-schema.md @@ -0,0 +1,32 @@ +# The wire format stays flat and deliberately does not mirror the schema + +Status: accepted + +Storage splits a tracked series across two tables (ADR-0003), but `GET /bookmarks` and +`PUT /bookmarks/{key}` keep emitting and accepting one **flat** JSON object with `title`, +`cover`, `last_chapter` and `latest_chapter` as siblings — exactly the shape they had +when there was one table. The server joins on the way out and decomposes on the way in. + +## Why a future reader will find this surprising + +The obvious move after splitting a table is to nest the JSON to match. Don't "fix" this. + +**A nested payload would have broken every installed userscript instantly.** Scripts read +`b.title` directly; moving it to `b.series.title` yields `undefined` — no error, just +blank rows and a New Chapter signal that silently reports nothing forever. Because +Violentmonkey updates roughly once a day per device, the migration relies on old scripts +continuing to work during a 14-day grace window. A nested format and that grace window +are mutually exclusive. + +**It is also the better contract independently of compatibility.** A client rendering one +row needs the title and the reading position together; nesting exports the re-stitching +to every browser to mirror a decision about disk layout it should not know about. Keeping +them separate lets storage change again later without a client release — which is the +whole reason this ADR is worth the paragraph. + +## Consequence + +The flat shape is a contract, not an implementation detail. Changing the storage schema +must not change it. It follows the rule already in force for `updated_at`: the server +owns the truth and returns the row **as stored**, and clients adopt the response rather +than their own payload. diff --git a/docs/agents/domain.md b/docs/agents/domain.md new file mode 100644 index 0000000..ca179ff --- /dev/null +++ b/docs/agents/domain.md @@ -0,0 +1,47 @@ +# Domain Docs + +How the engineering skills should consume this repo's domain documentation when exploring the +codebase. Layout: **single-context** — one `CONTEXT.md` plus `docs/adr/` at the repo root. + +## Before exploring, read these + +- **`CONTEXT.md`** at the repo root — the glossary / ubiquitous language. +- **`docs/adr/`** — read ADRs that touch the area you're about to work in. + +If any of these files don't exist, **proceed silently**. Don't flag their absence; don't suggest +creating them upfront. The `/domain-modeling` skill (reached via `/grill-with-docs` and +`/improve-codebase-architecture`) creates them lazily when terms or decisions actually get resolved. + +Neither exists yet in this repo. The existing `AGENTS.md` / `CLAUDE.md` and `docs/design-system.md` +carry the current architecture and design law — read those regardless. + +## File structure + +``` +/ +├── CONTEXT.md +├── docs/adr/ +│ ├── 0001-....md +│ └── 0002-....md +├── backend/ +└── userscript/ +``` + +If this repo ever splits into genuinely separate contexts, add a root `CONTEXT-MAP.md` pointing at +one `CONTEXT.md` per context and update this file. + +## Use the glossary's vocabulary + +When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a +test name), use the term as defined in `CONTEXT.md`. Don't drift to synonyms the glossary +explicitly avoids. + +If the concept you need isn't in the glossary yet, that's a signal — either you're inventing +language the project doesn't use (reconsider) or there's a real gap (note it for +`/domain-modeling`). + +## Flag ADR conflicts + +If your output contradicts an existing ADR, surface it explicitly rather than silently overriding: + +> _Contradicts ADR-0002 (…) — but worth reopening because…_ diff --git a/docs/agents/issue-tracker.md b/docs/agents/issue-tracker.md new file mode 100644 index 0000000..d0ba10a --- /dev/null +++ b/docs/agents/issue-tracker.md @@ -0,0 +1,60 @@ +# Issue tracker: Gitea (`tea` CLI) + +Issues and specs for this repo live as issues on the self-hosted Gitea instance +`gitea.violetcrown.my.id` (repo `sulthan/mangaBookmark`). **`gh` does not work here** — use +[`tea`](https://gitea.com/gitea/tea) for everything past plain git. Auth lives in `tea login`, +not a `GH_TOKEN` env var. `tea` infers the repo from the local clone's `origin`. + +`tea` prints rendered boxes rather than plain text; pass `--output json` (or `-o json`) when a +skill needs to parse the result. + +## Conventions + +- **Create an issue**: `tea issue create --title "..." --description "..."` (`--labels`, + `--assignees` optional). Multi-line bodies: pass the body through a shell variable or heredoc. +- **Read an issue**: `tea issue --comments` (add `-o json` for machine-readable output). +- **List issues**: `tea issue list --state open -o json --fields index,title,body,labels,state,author`; + filter with `--labels "..."`, `--state open|closed|all`, `--assignee`, `--keyword`. +- **Comment**: `tea comment "..."` (alias of `tea comments add`). +- **Apply / remove labels**: `tea issue edit --add-labels "..."` / `--remove-labels "..."`. + Labels must exist first — see `tea labels list` / `tea labels create --name "..." --color "#rrggbb"`. +- **Close**: `tea issue close ` (comment separately with `tea comment`; `close` takes no + `--comment` flag). + +## Pull requests as a triage surface + +**PRs as a request surface: no.** _(Set to `yes` if this repo treats external PRs as feature +requests; `/triage` reads this flag.)_ + +When set to `yes`, PRs run through the same labels and states as issues, using the `tea pr` +equivalents: `tea pr --comments`, `tea pr list --state open -o json`, +`tea pr create --head --base main --title "..." --description "..."`, `tea comment`, +`tea pr close`. Gitea shares one index space across issues and PRs, so a bare `#42` may be either +— resolve with `tea pr 42` and fall back to `tea issue 42`. + +## When a skill says "publish to the issue tracker" + +Create a Gitea issue with `tea issue create`. + +## When a skill says "fetch the relevant ticket" + +Run `tea issue --comments`. + +## Wayfinding operations + +Used by `/wayfinder`. The **map** is a single issue; **tickets** are child issues. + +- **Map**: one issue labelled `wayfinder:map` holding the Notes / Decisions-so-far / Fog body. + `tea issue create --labels wayfinder:map --title "..." --description "..."`. +- **Child ticket**: an issue labelled `wayfinder:` (`research`/`prototype`/`grilling`/`task`) + with `Part of #` as the first body line, and a task-list entry in the map body. `tea` has no + sub-issue command, so the task list plus the `Part of` line is the canonical link. +- **Blocking**: a `Blocked by: #, #` line at the top of the child body. Gitea's native issue + dependencies exist in the API but `tea` does not expose them; the body line is the source of + truth. A ticket is unblocked when every listed blocker is closed. +- **Frontier query**: `tea issue list --state open -o json` scoped to the map's task list; drop any + ticket with an open blocker or an assignee; first in map order wins. +- **Claim**: `tea issue edit --add-assignees ` — the session's first write. + (`tea` has no `@me` shorthand; use the Gitea username from `tea login list`.) +- **Resolve**: `tea comment ""`, then `tea issue close `, then append a context + pointer to the map's Decisions-so-far via `tea issue edit --description "..."`. diff --git a/docs/agents/triage-labels.md b/docs/agents/triage-labels.md new file mode 100644 index 0000000..d068ec1 --- /dev/null +++ b/docs/agents/triage-labels.md @@ -0,0 +1,20 @@ +# Triage Labels + +The skills speak in terms of five canonical triage roles. This file maps those roles to the actual +label strings used in this repo's issue tracker (Gitea — see `docs/agents/issue-tracker.md`). + +| Label in mattpocock/skills | Label in our tracker | Meaning | +| -------------------------- | -------------------- | ---------------------------------------- | +| `needs-triage` | `needs-triage` | Maintainer needs to evaluate this issue | +| `needs-info` | `needs-info` | Waiting on reporter for more information | +| `ready-for-agent` | `ready-for-agent` | Fully specified, ready for an AFK agent | +| `ready-for-human` | `ready-for-human` | Requires human implementation | +| `wontfix` | `wontfix` | Will not be actioned | + +When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label +string from this table. + +Gitea will not auto-create labels on `tea issue edit --add-labels`; create a missing one first with +`tea labels create --name "