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

• 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.