Files
bookhoard/AGENTS.md
T
John O'Keefe ee98c0fb3b
Release / build-and-push (push) Successful in 3m29s
docs(dev): test-running policy + media-fixture rules; ignore testdata
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).
2026-10-19 07:54:55 -04:00

11 KiB
Raw Blame History

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.

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.

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