bookhoard/AGENTS.md

# AGENTS.md — Bookhoard server

Standing rules for any agent or developer working in this repo.
Project-wide context lives in the sibling repos' AGENTS.md files
(../bookhoard-app/AGENTS.md is the Android client's).

## Rendering ownership (templ vs TypeScript) — ALWAYS apply

The point of templ + htmx + Alpine is: the server renders the UI, and
JavaScript is kept minimal. The rule that keeps it that way:

> **JavaScript decides what shows. templ decides what it looks like.**

- Content derived from **server data** (DB rows, search results, match
  lists, conflict cards, any list/card/badge/table) is rendered by
  templ. When it changes client-side, fetch a **templ partial** from an
  SSR route and swap it in (`htmx.ajax(...)`), never rebuild it from a
  JSON payload with `innerHTML` string templates.
- **Pure functions of client-local state** (open/closed panels, tabs,
  password strength from a typed value, toasts) may render in
  TS/Alpine — no server round trip needed.
- `web/src` is **TypeScript only** — no new `.js` source files.
  Vendored third-party and vite build output under `web/static` are
  exempt.
- Inline `<script>` in a `.templ` file needs a reason a bundled module
  cannot do the job (pre-paint execution). Currently sanctioned:
  `theme_flash_guard.templ` only.

Sanctioned JS islands (engines, not UI renderers — exempt from the
above): the reader (foliate-js + `web/src/reader/`), Chart.js graphs in
stats, toasts, password validation. Anything else that renders app UI
in TypeScript is a defect; if a templ twin of it exists, one of the two
must be deleted.

**Documented exception — dashboard sections**: `renderDashboardCollections`
/ `renderSectionHTML` in `web/src/dashboard.ts` re-render dashboard
sections from `/api/dashboard/sections` JSON (library switch, settings
save, refresh button) to preserve the switch fade/transition on a page
with heavy interactivity. First-paint section markup is still templ
(Dashboard handler in `internal/router/frontend.go`). This exception is
deliberate; do not clone it to new pages, and do not "fix" the dashboard
without restoring a templ-partial fade path first.

## Library visibility (family-lens rule) — ALWAYS apply

Per-user library visibility (`library_visibility` table, default-ALLOW:
every user sees every library unless an admin hides one) must be
enforced on **every endpoint that returns media metadata or content**.
It is NOT automatic — enforcement is per-handler.

- **New endpoints that list/filter media or series** MUST filter by the
  requesting user's visible-library set, and MUST reject an explicit
  `library_id` outside that set with 403 (never silently pass).
- Helpers (do not hand-roll):
  - `handlers`: `(*MediaHandler).visibleLibrarySet` /
    `(*SeriesHandler).visibleLibrarySet` — membership map + id slice.
  - `router`: `visibleLibraryIDsFor` — same for SSR routes.
  - `resolveLibrary` (helpers.go) only honors requested library ids
    inside the visible set.
- SQL pattern: `AND (sqlc.narg('visible_library_ids')::uuid[] IS NULL OR
  library_id = ANY(sqlc.narg('visible_library_ids')::uuid[]))` — see
  `ListMediaItems`, `GetDistinctSeries`, `SearchMediaItems`.
- Content endpoints (file bytes, page images) additionally gate per item
  like `ReaderHandler.ensureItemVisible` / the download endpoint.
- Tests must pin visibility: hidden library → 403 on the endpoint, and
  absent from unscoped aggregates.

History: 2026-10-13 (`431fec0`) the series/media listing surfaces
leaked hidden libraries to any authenticated account — series endpoints
were name-only across ALL libraries. Fixed; do not regress.

## Tests

- **THE DOCTRINE — tests are never run against a real/production
  server.** The dev DB is the test environment, and the invariant every
  test must uphold is: **leave the testuser experience alone** — its
  settings, scans, and reading data must be exactly as the suite found
  them. The helpers exist to enforce this; if a test needs protection
  the helpers don't provide, EXPAND THE HELPERS rather than hand-rolling
  (2026-10-21: `setSystemSettingRestored` — snapshot-and-restore for
  system_settings — exists because a hardcoded ev restore could clobber
  a deliberate dev bump, and the live dev server obeys that setting on
  its next poll).
- Use `setupTestServer()` from `cmd/server/tests/test_helpers_test.go`
  (see the Conventions section below for the pool/cleanup rules).
- Integration tests run against the SHARED dev database: never write
  destructive SQL without preserving the dev rows (libraries Manga/
  Comics/Ebooks, testuser's annotations and progress are real data).
- Test-created users must use `@tests.bookhoard.internal` or
  `@example.com` emails and "test"-named collections/libraries so the
  harness cleanup can find them.
- **NEVER rename or edit the shared admin (`testuser`) in tests** — a
  renamed admin escapes the cleanup preserve-list and cascades real
  data (this wiped the dev annotations once; see user_test.go
  TestUpdateProfileSelfEdit for the dedicated-user pattern).
- **`testuser` is a LIVE reader**: the dev account genuinely reads in
  the web reader while the suite runs, so `reading_history` /
  `reading_progress` rows accrue on it mid-test. Exact-total
  assertions against that account flap (three stats/analytics tests
  broke on 2026-10-14 within hours of session capture shipping).
  Tests that need deterministic data use `setup.RegularToken` +
  `getRegularUserID` (the fresh per-test user), or assert ranges
  instead of equals. Related: **no table-wide DELETEs in test
  cleanups** — sync_integration's `DELETE FROM reading_history WHERE
  true` destroyed the live reader's stats data on every suite run
  until scoped to the test-sync users (cascade via user delete).
- **RUN ONLY YOUR OWN TESTS while iterating**: integration tests
  require the containerized dev DB — a bare host `go test
  ./cmd/server/tests` fails on auth on every machine not running the
  compose stack. Use the filtered container runner
  (`docker compose -f docker-compose.yml -f
  docker-compose.dev.yml --profile tests run --rm tests go test
  ./cmd/server/tests -run 'TestYourPrefix' -v`) and save full-suite
  runs for pre-commit validation; several tests read live shared
  state (system_settings, real library folders, the periodic backup
  scan's websocket broadcasts) and flap on data/timing your change
  never touched. Full recipes: docs/contributing/development.md
  → Testing.
- **NEVER COMMIT BINARY MEDIA** (mp3/m4b/flac/... fixtures): 
  `internal/services/testdata/` is gitignored by policy; generate
  media at test runtime with ffmpeg instead (test images ship it).
  ffmpeg cannot write Audible freeform atoms — those paths are unit-
  tested with synthetic ffprobeReport values. Recipes:
  docs/contributing/development.md → Media fixtures.

## Web (TypeScript) tests

- `npm run test` (vp test) runs the vitest suite: colocated `web/src/*.test.ts`
  files, jsdom environment (configured in vite.config.ts `test`). Do NOT switch
  to happy-dom — DOMPurify's sanitizer output is mangled under it, which makes
  the markdown security suite test nothing (verified empirically; jsdom is the
  documented fallback for exactly this reason).
- Static gates: `npx tsc --noEmit`, `npx vp check web/src` (format + lint),
  `npx vp fmt web/src` to apply formatting.
- Tests are never bundled (build entries are main.ts/reader.ts only). When
  touching build config, re-prove it: back-to-back `npm run build:ts` runs must
  produce byte-identical hashes, and no test-only strings may appear in
  web/static.
- markdown.test.ts pins the notes XSS defenses (FORBID_TAGS, scheme checks,
  parser-level html escaping). Loosening any assertion there is a security
  decision, not cleanup.

## Timezones

- The server owns display conversion: clients send instants (RFC3339),
  the server renders in the requesting user's timezone.
- Use `utils.FormatTimeInTimezone` / `utils.UserLocation` — never
  `time.Now().Format(...)` bare (that is server-UTC).
- Analytics day-buckets must bucket in the user's timezone
  (`CreatedAt.In(loc)`), and date-range end bounds must be end-of-day.

## Debug discipline (test-failure triage) — 2026-10-17

When a test or bruno request fails, suspect the TEST FIRST — most
failures in this repo are harness/schema/environment drift, not server
bugs (2026-10-17 bruno sweep: drifted body keys, bodies pre-truncated
at their first `{{var}}`, missing `/api` prefixes; host runs hitting
container-only paths/hostnames). Triage order: (1) read the
test/fixture/payload schema and diff against a known-good sibling
(bruno: the `NewDevDBSetup` files are the reference for `body.data`
form); (2) check environment assumptions (host vs container,
harness cleanup, live-DB traffic); (3) cheap probe (curl, one SQL
query) before any deep dive.

But test changes are ASYMMETRIC with code changes: server code may be
fixed freely when it is the bug; test/contract edits may only
(a) fix harness plumbing, (b) WIDEN pins additively with a
`docs/contract_pins.md` changelog entry, (c) delete tests of deleted
behavior — never narrow an assertion or flip an expectation to match
observed behavior. Contract pins were green at a known commit, so git
history arbitrates "test stale vs code changed." If a test looks right
but fails and no harness bug explains it, that is prima facie evidence
of a server bug: stop and flag the user.

## Known gotchas (do not relearn)

1. **templ does NOT interpolate `{ expr }` inside quoted attribute
   values** — it emits the literal text. `x-init="f('{ col.ID }')"`
   shipped a request to `/api/collections/{ collection.ID }` found by
   the request logs; the whole attribute value must BE an expression:
   `x-init={ "f('" + col.ID + "')" }`. (Every occurrence of this bug —
   including `class="theme-{ user.Theme }"` on 22 page bodies and 26
   attribute interpolations — was swept and fixed in 2026-10; when
   adding attributes with dynamic values, the whole value is an
   expression, and free text goes through `templates.JSONArg`.)
2. **SettingsRegistry caches**: writes must go through the settings
   API (Reload on save). Direct SQL writes to system_settings leave
   the cache stale until the next API-path write — never "preview"
   rule changes via psql and expect them applied (2026-10-14: a live
   curl test flip looked ignored until the API path was used).
3. **User collections are not dashboard query types** (amended 2026-10-19,
   A2c): user collections ARE dashboard sections by default now —
   GetUserCollectionsForDashboard no longer gates on show_on_dashboard
   (which remains system-section provisioning state; hiding is
   dashboard-prefs `hidden_collections` territory). GetDashboardSections
   still skips genuinely empty user sections, but hide_finished
   collections emit a Complete section instead of vanishing. Collection
   detail (JSON + SSR) must still NOT route user rows through the
   dashboard-section path — a 7ce7321 regression did exactly that
   (static detail without library_id returned zero books until 0a7fbd6).
   Only real system query types (continue-reading etc.) route there;
   `query_type='filter'` is the schema DEFAULT on every user row, not a
   query type.
4. **End-of-day arithmetic**: anchor the END bound to the START of the
   day, never to NOW(). `now.AddDate(0,0,1) − 1ns` carries this
   instant's clock time into tomorrow — a range "ending today"
   resolves tomorrow's calendar date and today's rows land in
   yesterday's bucket (found twice now: analytics 1601548 and stats
   2026-10-14). Same trap, two deaths.
5. **Test binaries run against the LIVE dev DB**: hand-probes made in
   psql during debugging (seed rows, fake sessions) will silently
   pollute the next integration run — DELETE them when done (this
   burned the first stats suite run: a stray 30-min probe row shipped
   a phantom "185 minutes" failure).
6. **SaveProgressRequest.DeviceID is NOT always a device**: the
   web/app progress paths have historically stuffed the USER id into
   it (it only fed a broadcast label before stats). Anything that
   dereferences it (reading_history device_id FK, device joins) must
   probe DeviceExistsForUser first — stats capture does.

## Conventions (absorbed from the retired PROJECT_GUIDELINES.md —
## only the rules that still matched reality)

- **Runtime is Docker** (Makefile auto-detects docker, falls back to
  podman). The old "podman only" rule is dead.
- **The server binary is only built inside the Docker build**
  (`make rebuild-app`); host-side Go builds are for tests/vet only.
- **Schema changes go in place** — edit `database/schema/schema.sql`,
  never create migration files, until a release splits them.
- **Business logic lives in services**, not handlers; handlers parse
  requests and call services (all DB access goes through services).
- **Integration tests**: one `setupTestServer()` per test function;
  test pools use `max_conns=1`; never create a `pgxpool` per subtest
  (connection exhaustion); use `t.Cleanup()`-based helpers; table-driven
  subtests over duplicate functions.
7. **Scanner re-extraction is write-after-success** — a failed/empty
   extraction must NEVER clear scan-derived columns. `updateMediaItem`
   (generic full-row path) preserves `existing.AudioMetadata` when the
   new extraction carries no audio payload, and `updateAudioBookItem`
   refuses empty payloads over populated rows (both guarded 2026-10-21
   after an ev-gated re-extract NULLed every multi-file audiobook's
   audio_metadata — books displayed as 1 second long and lost their
   track manifests). New scan-owned column writes need the same guard.
8. **templ double-escapes style attribute values** — the codegen wraps
   style exprs in a sanitizer AND `templ.EscapeString`, so ANY quote
   character (even inside `templ.SafeCSS`) reaches the DOM as literal
   `&#39;` text and the CSS never parses (2026-10-21: every book-detail
   hero backdrop broken + Firefox fetched `/media/&` → 500 spam).
   Emit quote-free CSS: unquoted `url(...)` tokens via `SafeCSS` +
   `cssURL()` in book_detail.templ.
9. **`vp test` runs the vite config** — plugins with build hooks fire
   in the test context too. `cleanStaleChunks` guards on
   `produced.size === 0` precisely because a bare sweep deleted every
   hashed chunk in `web/static` when run under vitest (2026-10-21:
   `npm run test` gutted the build; every page 404'd its imports).
   Any future outDir-mutating plugin needs the same "am I a build?"
   guard.