Integration tests require the containerized dev DB and bind-mounted paths — a bare host `go test ./cmd/server/tests` fails on database auth on every machine not running the compose stack (the old doc prescribed exactly that flow). development.md now documents the container runner and the iteration rule: run ONLY your own tests via a filtered container invocation while working, and reserve 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 or timing a change never touched. Binary media fixtures are never committed: internal/services/ testdata/ is gitignored by policy (the audio metadata tests read real-format binaries that ffmpeg cannot synthesize — Audible freeform atoms — so they live untracked on the machines that run the suite). The new "Media fixtures" section carries the ffmpeg recipes for every supported container, tag writing, embedded covers and ffmetadata chapters, plus the manual end-to-end recipe for the live system (one book per subfolder — a directory with ≥2 audio files ingests as one folder book).
11 KiB
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 withinnerHTMLstring 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/srcis TypeScript only — no new.jssource files. Vendored third-party and vite build output underweb/staticare exempt.- Inline
<script>in a.templfile needs a reason a bundled module cannot do the job (pre-paint execution). Currently sanctioned:theme_flash_guard.templonly.
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.
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_idoutside 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[]))— seeListMediaItems,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()fromcmd/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.internalor@example.comemails 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). testuseris a LIVE reader: the dev account genuinely reads in the web reader while the suite runs, soreading_history/reading_progressrows 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 usesetup.RegularToken+getRegularUserID(the fresh per-test user), or assert ranges instead of equals. Related: no table-wide DELETEs in test cleanups — sync_integration'sDELETE FROM reading_history WHERE truedestroyed 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/testsfails 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.
Timezones
- The server owns display conversion: clients send instants (RFC3339), the server renders in the requesting user's timezone.
- Use
utils.FormatTimeInTimezone/utils.UserLocation— nevertime.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)
- 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 — includingclass="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 throughtemplates.JSONArg.) - 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).
- User collections are not dashboard sections: nothing sets
collection show_on_dashboard for user rows, and GetDashboardSections
skips empty user sections. Collection detail (JSON + SSR) must NOT
route user rows through the dashboard-section path — a
7ce7321regression did exactly that (static detail without library_id returned zero books until0a7fbd6). 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. - End-of-day arithmetic: anchor the END bound to the START of the
day, never to NOW().
now.AddDate(0,0,1) − 1nscarries 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: analytics1601548and stats 2026-10-14). Same trap, two deaths. - 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).
- 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 usemax_conns=1; never create apgxpoolper subtest (connection exhaustion); uset.Cleanup()-based helpers; table-driven subtests over duplicate functions.