Files
sulthan f5d3fe58ec Add a gitea skill; point AGENTS.md at it (#126)
Forge usage lived in three places (`AGENTS.md`, `docs/agents/issue-tracker.md`, and habit). This moves the how-to-run-`tea` half into a model-invoked skill that fires on any issue/PR task, and reduces `AGENTS.md` to identity plus pointers.

- **new** `.claude/skills/gitea/SKILL.md` — command table plus the traps `tea <cmd> --help` will not tell you.
- `AGENTS.md` — Forge section is now one line: Gitea not GitHub, `gh` and the `issue://`/`pr://` URIs fail, then pointers to the skill, `docs/agents/issue-tracker.md`, and `docs/agents/triage-labels.md`.
- `.claude/skills/implement-tickets/SKILL.md` — pointer split: tracker conventions to the doc, `tea` usage to the skill.

Both `docs/agents/` files are untouched; the skill cites them instead of restating them.

Facts in the skill are measured against `tea` 0.14.2 on 2026-08-17, not remembered:

- `gh` is not installed, so `read issue://71` errors — there is no fallback to add.
- **A bare read is a truncated read.** Without `--comments`, `tea issue <n>` drops every comment silently, with no prompt under a non-TTY: issue #123 prints 40 lines bare, 132 with the flag. The skill makes `--comments` mandatory for any read meant to understand a ticket, with `tea issue list --fields index,comments` as the checkable count.
- Issues and PRs share one index space; output is rendered boxes so parsing needs `-o json`; `close` takes no `--comment`; labels never auto-create; multi-line bodies need a heredoc; `tea` exposes neither sub-issues nor dependencies.

Unmeasured and marked as such: whether `--comments` covers a PR's review-comment stream — no PR in this repo has comments, so `tea pr review-comments <n>` is named without a claim about overlap.

Reviewed-on: #126
Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com>
Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
2026-08-17 17:52:16 +07:00

194 lines
18 KiB
Markdown

# 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).
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.