Files
mangaBookmark/AGENTS.md
sulthan e1ba7fdabb fix: pgtest left an anonymous volume behind on every run (#175)
## Problem

`go test ./...` leaked one Docker volume per test package. `pgtest.Main` tore its throwaway container down with `docker rm -f` — no `-v`. The `postgres:17-alpine` image declares a `VOLUME` for `/var/lib/postgresql/data`, and an explicit `rm` without `-v` orphans that anonymous volume even though the container was created with `--rm`.

Found in passing: a 13-hour-old orphaned pgtest container (AutoRemove, `fsync=off`, one mount) still running from a killed test binary — the same leak's other half. Removed on the dev machine.

## Change

- `backend/internal/pgtest/pgtest.go`: `-v` on all three `docker rm -f` teardown sites (`Main`'s defer, and both error paths in `start`).
- `AGENTS.md`: cleanup expectation under Commands — `docker compose down -v` for a stack, `docker rm -f -v` for a hand-run container, then check `docker volume ls` and `docker system df`. Explicitly out of bounds: removing the user's `postgres-data` volume, or a blanket `docker system prune` of their images and build cache.

## Verification

`docker volume ls` snapshot diffed across a real `./internal/store` run: no new volumes, `docker system df` reports 0 local volumes.

Reviewed-on: #175
Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com>
Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
2026-08-23 13:14:49 +07:00

19 KiB

AGENTS.md

Repo-wide guidance for coding agents.

What this is

Read-progress tracker for two libraries — manga and novels — behind one self-hosted Go backend. Two separate Violentmonkey userscripts inject on-page UI (floating button + slide-in panel) and sync progress, so bookmarks unify across sites and devices:

  • manga-bookmark.user.js — asurascans.com (asuracomic.net is dropped: its deep links 301 to the asurascans.com root, discarding the path), demonicscans.org, comix.to, kagane.to.
  • novel-bookmark.user.js — novelfull.com, lightnovelworld.net.

One backend, one bookmarks table: a kind column (manga|novel) splits the libraries and the web UI switches between them. Rows are keyed <site>:<series_id>.

Hard constraints (drive design — don't violate)

Nothing below is derivable from reading the code — it is why the code looks the way it does, plus dated measurements against services we don't control.

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).
  • Every site is its own origin with its own localStorage — a shared remote store is the 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 is per-zone configuration plus request fingerprint, not IP reputation — 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 — a Site can turn its protection on overnight, which is exactly what comix.to did on 2026-08-12. An earlier version of this line blamed "Cloudflare's bot scoring"; that was wrong. The 1-99 bot score is Enterprise Bot Management only and does not exist for a free-plan zone, and no per-IP request rate is documented as an input to challenge issuance. Backend fetcher still needs graceful-degrade path for when challenged, and adapters should be verified against live pages (Playwright MCP, on-device devtools, direct probe) before finalizing, not assumed from single earlier test.
  • kagane.to, comix.to and novelfull.com are the exception to the above — all three sit behind a Cloudflare JavaScript challenge no TLS fingerprint clears, so the backend polls them over CDP (BROWSER_WS_URL). When that's unset, kagane and comix are skipped entirely (a plain fetch would only retrieve a challenge page) while novelfull pages are still attempted over plain TLS — its challenge is a live time-varying fact and its cover bytes never need the browser. comix turned hostile on 2026-08-12: its cover host static.comix.to is gated too, so its cover bytes go through the browser as well, and its page is read as an in-tab fetch() of the series URL rather than a rendered DOM — comix is an SPA, and rendering costs ~65 requests for the same server-rendered HTML one fetch returns. The three other sites poll fine over plain TLS.
  • The CDP browser must look like a real browser, and stock headless images don't. Measured 2026-08-08 against kagane.to, all from the same IP: chromedp/headless-shell:stable never cleared the challenge in 90s (navigator.webdriver true, empty plugin list, Chromium-branded client hints — suppressing webdriver alone changed nothing); zenika/alpine-chrome ships Chrome 124, refused outright; real Chrome with the default --headless=new UA never cleared, because the UA says HeadlessChrome; real Chrome with a stock UA and a non-UTC clock zone cleared in ~4s. Hence chrome/ — a Debian image with google-chrome-stable, a version-derived UA, and TZ/BROWSER_TZ. Chrome reads the zone name through ICU from /etc/localtime's symlink target, ignoring the file's contents, so mounting the host's /etc/localtime does not work; /etc/timezone is mounted instead.
  • The browser is not in the API stack and must not be put back. It's its own compose unit (chrome/docker-compose.yml) on a second machine, reached over the tailnet — it held 471 MiB on a 1974 MiB swapless VPS, and a residential egress avoids the cloud-hosting-IP signature Bot Fight Mode documentedly challenges (not a better "score" — free-plan zones have no score). Consequences that constrain code: BROWSER_WS_URL must be a tailnet IP (a MagicDNS name 500s at /json/version, same trap as the old Docker service name); the CDP port binds to the tailnet address only, since CDP authenticates nothing and that host has a real LAN; and the browser is on-demand, so an unreachable or asleep one must degrade exactly as an unset BROWSER_WS_URL — plain-TLS libraries unaffected, kagane/comix logged and skipped, stored covers still served. Never add chromedp.NoModifyURL: discovery per fetch is what makes a restarted Chrome invisible.
  • UTC is the tell, not a country mismatch. A UTC clock is the datacenter default, and the challenge refuses it; any real zone clears. Measured 2026-08-08, identical container, one Indonesian egress IP: UTC never cleared in 60s (twice), while Asia/Jakarta and America/New_York both cleared in 4s. An earlier note here claimed the zone had to match the egress IP's country — that was wrong, inferred from the host clock (Asia/Bangkok) rather than the measured egress. A second earlier claim, that Cloudflare "scores" a UTC clock, was also wrong: the measurement is real but the mechanism is not documented anywhere — Cloudflare publishes no timezone signal, and free-plan zones carry no score at all. BROWSER_TZ therefore needs a plausible zone, not a geolocated one.
  • A challenged page needs the tab kept open. The interstitial takes seconds to solve and only then writes clearance into the browser's shared cookie jar. Navigate-read-close never clears anything; BrowserFetcher.run holds one tab and re-reads until the payload arrives.

Architecture

Two Violentmonkey userscripts (isolated world, per-site adapters, localStorage cache)
   -- fetch() HTTPS -->  reverse proxy (TLS + CORS)  -->  Go net/http  -->  Postgres (volume)
                                                              |
                                                              | CDP over tailnet
                                                              v
                                            on-demand Chrome, separate machine (chrome/)

Two deployable units on two machines: the API stack (docker-compose.yml + docker-compose.prod.yml, on the VPS) and the browser (chrome/docker-compose.yml, on the home machine). They share nothing but BROWSER_WS_URL and update independently. Backend-specific detail lives in backend/AGENTS.md, userscript-specific detail in userscript/AGENTS.md.

Commands

Backend (cd backend):

  • 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 (bookmark-api + postgres only; postgres-data named volume, restart: unless-stopped). No browser — without BROWSER_WS_URL the poller logs and skips kagane and comix. To run one: cd chrome && BROWSER_BIND_ADDR=172.17.0.1 docker compose up -d --build, then BROWSER_WS_URL=ws://172.17.0.1:9222 in the root .env (bridge gateway, so the API container can name it by IP).

Clean up Docker after testing. Storage on the dev machine is scarce, so anything you started for a test you also tear down before calling the work done: docker compose down -v for a stack you brought up, docker rm -f -v for a container you ran by hand (-v, or the image's anonymous data volume survives). Then check docker volume ls and docker system df for leftovers and reclaim them — a dangling volume nobody notices is the leak that fills the disk. Never remove the postgres-data volume of a stack the user is actually running, and never blanket-docker system prune their images or build cache.

Live CDP proof (needs that browser and network, skipped otherwise): SMOKE_BROWSER_WS_URL=ws://<ip>:<port> go test -run 'TestSmokeKagane|TestSmokeComix' ./internal/latest — fetches a real kagane and comix cover and chapter list. A red run means the challenge is not clearing from this IP, which is a live fact to re-check, not necessarily a defect. A red TestSmokeComix reporting ERR_CERT_COMMON_NAME_INVALID is not the challenge: it means the resolver the browser container uses hijacks comix.to. Observed 2026-08-16 on one Indonesian ISP, which CNAMEs it to a block page (aduankonten.id). Check with docker exec <browser> getent hosts comix.to, and if it is hijacked, run the container with --add-host comix.to:<ip> --add-host static.comix.to:<ip> from a DoH lookup (curl -H 'accept: application/dns-json' 'https://1.1.1.1/dns-query?name=comix.to&type=A'). Machine-local, so don't put those hosts in chrome/docker-compose.yml.

Smoke test: curl endpoints with Authorization: Bearer <token>; confirm OPTIONS preflight return CORS headers and /healthz return 200.

Forge: Gitea, not GitHub

origin is self-hosted Gitea (gitea.violetcrown.my.id, repo sulthan/mangaBookmark), so gh don't work here and the issue:///pr:// URIs error out — drive the forge with tea. How to run it — commands, traps, JSON output: skill gitea. Tracker conventions (ticket bodies, wayfinding, PR-as-request-surface flag): docs/agents/issue-tracker.md. Triage label strings: docs/agents/triage-labels.md.

Design system

Web UI + userscript panel follow Cinder. Tokens are the :root block in backend/internal/web/static/style.css; that file, backend/internal/web/templates/*, and the userscript TEMPLATE/CSS are the only places it is expressed. Source of truth for the visual language is the Claude Design project BookmarkManager Web UI (969ac210-fe02-4c01-ae1b-9a271dcc779a).

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 a series out of the list (archive/finish/remove) must be confirm-gated via its own .confirm-row; only restore fires instantly.

Security invariants

Existing guarantees — don't regress:

  • Auth on /bookmarks*: require Authorization: Bearer <credential> — the acting Reader's credential, matched by SHA-256 against readers.token_sha256 — constant-time compare (via the hash, never the secret itself), 401 otherwise.
  • CORS: reflect Origin only when in ALLOWED_ORIGINS; allow GET,PUT,DELETE,OPTIONS + headers Authorization,Content-Type; answer preflight OPTIONS with 204.

Secure coding rules (code you write here)

Anchored to OWASP Top 10 / ASVS. Every rule below already has a working example in-tree — match it, don't start a second convention. AI-written backends fail on exactly these: broken access control, injection, weak session/error handling, invented dependencies.

Go backend:

  • 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.
  • Compare secrets with hmac.Equal / subtle.ConstantTimeCompare, never ==. A credential is matched by the SHA-256 the readers table holds, which is already a fixed-width equality — a new secret comparison must not regress to ==.
  • Errors: generic text to the client (http.Error(w, "internal error", 500)), detail to log.Printf. Never log TOKEN_KEY, a Reader's credential, DISCORD_CLIENT_SECRET, a session id, or a whole Authorization header.
  • Proxy headers are trusted only where they already are: X-Forwarded-Proto for the Secure cookie flag, rightmost X-Forwarded-For for client IP (leftmost is attacker-supplied). Don't read either anywhere else.
  • Session cookies keep HttpOnly, SameSite, Secure-when-HTTPS; expiry is enforced by the sessions table lookup, not a signature.
  • Stdlib crypto only. No hand-rolled hashing, no MD5/SHA-1 anywhere security-bearing.
  • Validate at the handler boundary before storing: body capped by http.MaxBytesReader (64 KB), empty key and unknown status/kind rejected with 400. A bad value that reaches the store becomes every later reader's problem.

Userscript:

  • Site-derived and stored strings render via el(..., {text}) / textContent. {html} and innerHTML are for author-written literal markup only (TEMPLATE, CSS) — never a title, chapter label, or API response field. The page DOM belongs to a third-party site; treat it as attacker-controlled.
  • Isolated world protects the credential from the site's JS. It does not protect anything from an innerHTML sink you add yourself.
  • The userscripts carry __API_TOKEN__ placeholders, substituted at serve time with the requesting Reader's credential (internal/userscript). Never put a real credential in the repo, docs, commit messages, or issues. Rotation is a web-UI action (epoch bump, internal/token); TOKEN_KEY in backend env is what derives every credential — never log it.
  • fetch() targets API_BASE only — no dynamic origin, no site-supplied URL. authHeaders() goes nowhere but the backend.
  • localStorage is shared with the site's own JS: cache and queue live there, credentials never do.
  • Wrap every localStorage read/write and JSON.parse in try/catch (quota, private mode, corrupt entry), as the existing helpers do.

Dependencies: stdlib first; a new module needs a stated reason. Confirm a package actually exists before adding it — a plausible name may be fiction (~20% of LLM-proposed packages don't resolve, which is how slopsquatting lands). Pin exact versions.

Review gate: auth, CORS, session, crypto, and the fetch gate are security-critical. Editing one is not a drive-by change — say which invariant you preserved and run go test ./... before calling it done.

Comments

Comment only if code alone can't carry info. Cost per read — must earn spot. Wrong comment worse than none: it misleads readers and measurably degrades LLM performance on the file. Missing comment costs little. Bias to fewer.

Docstring on public/exported surface — exception, near-always worth it. Contract only: what it takes, returns, throws, mutates; units; pre/post conditions. Not a restatement of the body. Skip on private/obvious.

Inline — write for:

  • Why not what. Tradeoffs, non-obvious decisions, rejected alternatives.
  • heavy detail looking incidental — say so if "simplify" breaks it.
  • Non-local consequence, invisible from function alone.
  • Wire format / encoding / ordering / invariant — save callers re-deriving.
  • Gotcha/workaround, with ref (issue, RFC, vendor bug) 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 + owner.
  • Narrating the plan you just reasoned through. Plan in prose or in your head; ship the code, not the transcript.
  • Anything restating a name that could be fixed by renaming instead.

Staleness filter: if the comment describes something likely to change independently of this line, it will rot and start lying. Either anchor it to something stable, assert it in a test, or leave it out.

Style: one dense comment over a function beats one per line inside. Tight; no worked example unless the bug is subtle. On edit, update or delete stale comments in the code you touch — silence beats a lie.

Test: "competent reader get this from code in a few sec?" Yes → skip. Needs detour through another file/spec/git-blame/external doc → write it.

Writing an AGENTS.md

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.

Cite code, never docs, issues, or plans. A spec, ADR, plan file, or Gitea issue records what was true when it was written and then goes stale silently; an agent that follows the pointer reads a decision that may already have been reversed. Code is the only source true at read time — cite a package, file, symbol, env var, or route. The sole non-code exception is a sibling AGENTS.md. If a doc holds a fact an agent needs, restate the fact here rather than linking to it.

State a fact in prose only if the code cannot answer it. Split by derivability:

  • Structure — packages, routes, env vars, columns, struct fields. Rots fast, cheap to re-read. Name the symbol, write nothing else.
  • Mechanism — what a function does, how a flow proceeds. Name the symbol plus at most one line of orientation.
  • Rationale — why it is this way, what a "simplify" would break, what was tried and rejected. Not in the code and cannot be re-derived. Write it out.
  • Measurement — an observation against something we don't control. Write it out with the date; a dated fact is honest, an undated one pretends to be permanent.

Restating mechanism in prose is how these files rot: the code changes, the paragraph doesn't, and the next agent trusts the paragraph. A pointer degrades more honestly — and every symbol you name must actually exist, since a dead pointer is a bug, not a stale sentence.