The bucket name read as confusing — 'unsynced' implied data loss when
the push is always captured; it only lacks a device credential. The
bucket is just 'Web' now (service fallback, stats.ts fallback, panel
copy, tests).
Also: analytics_test exact-zero assertions (EmptyHistory,
NoReadingHistory) ran against testuser, which is a LIVE reader — real
sessions accrued on it mid-suite and both flapped. They now run under
the fresh regular user, like the stats tests; the 'testuser is a live
reader' rule is recorded in AGENTS.md.
- Attribution: registered clients may now send their device token in
X-Device-Token on PUT /api/media-items/:id/progress (the user JWT
stays primary). The token must map to a device OWNED BY the caller —
foreign/invalid/absent tokens soft-fail to plain web attribution so a
stats decoration can never break a sync. Attributed pushes capture
device_id (FK-safe), device_metadata.source = the registered device
name ('Pixel 9', 'Kindle Paperwhite'), and last_sync surfaces show
that device. Side effect worth having: app-vs-web pushes used to be
LWW-invisible (both sources read 'web'); they now surface as
legitimate cross-source progress conflicts when they diverge.
- /stats gains a By Device panel alongside By Library (grid 3-wide):
reading minutes + sessions + books per registered device, plus one
Web / unsynced bucket for device-less pushes so the panel always
reconciles with the headline totals. Range-aware, family-lens
scoped; renderBars shared between panels in stats.ts.
- The device identity IS the registered platform: devices rows already
carry the name/type the client registered (app sends Build.MODEL +
'mobile' at /api/devices/register, KOReader/Kobo likewise) — no new
identity plumbing on the server.
Tests: capture attributed to the device (row device_id + source
metadata + last_sync_source), foreign/nonexistent tokens soft-fail,
device buckets reconcile (80 attributed + 75 unsynced = 155 total).
Full suite PASS.
The design parked in the handoff (multi-tier stats off reading_history;
bookorbit-style in data, not looks) is in. The real prerequisite work
was data: reading_history existed but NOTHING wrote to it — the whole
stats feature was sitting on an empty table.
- CAPTURE (no client changes, no repo-boundary crossings): every
successful progress write through SaveProgress (KOReader + app + web
+ Kobo + offline queue — the one chokepoint) now records one
reading_history session row: span = the interval since the PREVIOUS
write instant, pages = positive page delta when both sides know the
page index, marker rows for first contact. Spans over 2h are kept as
zero-second markers (overnight suspend→resume, offline flushes must
not inflate totals) — the honest 'sync cadence, not a stopwatch'
caveat stays. device_id only when a REAL device row matches (the
web/app progress path historically stuffs the USER id into DeviceID
— probe instead of trusting it; FK violation would silently drop
every capture). Source preserved in device_metadata.
- API: /api/stats/summary?range=7|30|90|year (totals, zero-filled
daily arrays, streaks, most-active day/weekday, top books, library
breakdown — one call), /api/stats/this-week (widget: rolling 7d vs
previous + streak), /api/stats/books/:id (lifetime + recent
sessions). All bucketed in the user's timezone; finished ≡ any push
≥0.995; visible-library lens enforced at source. Legacy
/api/analytics/* kept untouched. Range windows anchor end-of-day to
the START of today (blasting the clock time into tomorrow resolved
'today' a calendar day late — found by the streak test).
- /stats page (sidebar Stats between Progress and Devices, previously
/analytics was an orphan): range selector, five stat cards, two
Chart.js v4 charts (daily minutes bar, pages/day line — chunked via
dynamic import so main.js stays untouched), per-library breakdown,
most-read list, empty-state explainer. /analytics now 301s to /stats.
The orphaned analytics.templ + analytics.ts removed (their containers
and the page JSON never matched; the page was astub).
- Dashboard 'This Week' widget (server-rendered, suppressed on service
failure) and the book detail 'Your Reading Stats' card (lifetime
totals of the viewer's own sessions) complete the tiers: per-user
default, per-library breakdown inline, per-book on the page,
per-system/admin aggregate parked as before.
Tests: statistics_test.go (capture semantics at the chokepoint,
tz/day buckets, zero-fill, streak math, finished detection, range
validation, auth gate, widget, per-book lifetime). Full suite PASS.
Type is DERIVED, not stored: a saved ruleset makes a collection SMART,
no rules = STATIC (stored collection_items links, unchanged behavior).
No schema change — collections.auto_assign_rules JSONB already carried
the config; user's decision 2026-10-14 (handoff: live-at-render, not
refresh-on-scan; create-time Static/Smart toggle; v1 NO hybrid).
- Rule gains an enabled flag (absent = enabled, legacy rules unaffected).
- Rule CRUD: POST/PUT(DELETE) /api/collections/:id/rules[/:ruleId] — the
survivor dead effort of the rules-builder UI finally has real
endpoints (rules were never editable after creation; nothing ever
re-evaluated them). First rule converts a user collection to smart;
removing the last reverts it to static. UpdateCollectionRulesJSON
edits ONLY the rules doc so rule CRUD cannot clobber metadata.
- Live evaluation: candidates from GetVisibleLiveMediaForRules —
visible-library set (family lens enforced at source) + archived IS
NULL AND missing_scan_count = 0 — any enabled rule matches, title
order. Zero refresh hooks by design: always current by construction.
- Membership mutations (add/remove/bulk-add) are 400 on smart — v1 no
hybrid; collection_items.excluded stays the reserved future hook.
- Rules builder page (/collections/:id/rules) first time ever routable;
templ whole-attribute interpolation ("{ expr }" inside a quoted attr
value is literal text, as theme-{ user.Theme } silently proved).
- UI: create modal Static/⚡Smart cards (smart create redirects into the
rules builder), ⚡ badge + real per-card counts on the grid (smart =
live matches, static = stored-link count), smart detail renders
matched books with no hand-pick actions + an Edit Rules button.
- API: GET /:id gains is_smart + live books; GET /: keeps the Android
app's default exclusion, include_auto=true surfaces smart rows with
live counts; TestRules previews against the SAME candidates the live
render uses (visible + live, was raw first-1000).
- FIXED pre-existing regression from 7ce7321: static collection detail
with NO library_id returned zero books — QueryType != '' caught user
rows ('filter' default) and routed them to dashboard sections that
never contain them; now only real system query types go there
(handlers + SSR). App-visible symptom: /api/collections/:id without
library_id came back books:[] for every static collection.
Tests: collections_smart_test.go (conversion, disabled rules, mutation
guards, revert-to-static, validation, system-collection prohibition,
include_auto semantics). Full offline integration suite PASS.
Three changes from the network-drive audit:
1. Delete-ordering fix. Deleting a media item cascades its annotations,
so the delete must never proceed while the item's library storage is
unreachable — the DB row died, the file survived as an orphan, and
the next scan re-imported it bare (annotations gone forever).
resolveMediaStorage stats the library's folders: reachable → delete
proceeds and the file is removed by ABSOLUTE path (the old code
passed a library-relative path to os.Remove, which never resolved —
file deletion has been a silent no-op); every folder unreachable →
409 with 'storage unreachable', row and annotations intact.
2. Latency: resolveLibrary stats every library folder on every SSR
render; folder reachability is now cached with a 30s TTL.
3. README: Network Storage section documenting the outage hysteresis
(mark → archive → auto-restore), the 90-day retention purge
(ARCHIVE_RETENTION_DAYS, 0 = never), refused deletes while storage
is down, and watch mode not firing on network mounts.
Integration tests: storage-down bulk delete refuses with the row
intact; real-file delete removes the file from disk.
default_timezone existed as a system setting (admin-editable, exposed
via SettingsRegistry.DefaultTimezone) but nothing read it at user
creation — the CreateUser query didn't even carry the column, so every
new user got the DB default UTC regardless. Both creation paths
(public register + admin create) now seed the user's timezone from the
setting, validated with time.LoadLocation and falling back to UTC when
unwired/empty/invalid.
Test-infra fixes surfaced by the run: TestUpdateProfileSelfEdit was
self-editing the SHARED admin (renaming it past the harness cleanup's
preserve-list, leaking a second admin row that broke the last-admin
tests) — it now edits a dedicated user with a unique name; the
last-admin tests get an ensureSingleAdmin precondition helper; the
websocket collection name is run-unique (non-test names persist in the
shared DB and UNIQUE(user_id,name) 409s on rerun).
The family-lens hardening: library_visibility was only enforced where
file bytes flowed (download/reader/OPDS) — metadata surfaces leaked
hidden libraries to any authenticated account, and registration was
open by default with no admin-side user creation.
- Series are grouped PER LIBRARY now (same name in two libraries =
two entries: manga adaptations / LN splits no longer merge);
/api/series aggregates only the requester's VISIBLE libraries and
/api/series/books REQUIRES a visible library_id (breaking contract;
web + app clients updated in the same change).
- /api/media-items, item detail, and the reader content endpoints
(chapters/panels/outline/THUMBNAILS — page images leaked unguarded
next to a gated GetPage) all enforce the visible set; resolveLibrary
no longer honors a hand-typed library_id outside the visible set.
- Annotation/note/bookmark mutations (update/delete/tombstone, sync
LWW paths incl. conflict resolution) carry ownership predicates:
another account's bare row id is a 404, not a silent delete.
- Admin user creation: POST /api/auth/users (AdminMiddleware) + a
Create User card in the admin Users UI — accounts stay creatable
with self-service registration OFF, which is now the schema default
(fresh installs; the first-admin bootstrap exception is unchanged).
- Tests: series contract re-pinned (per-library, visibility 403s),
new ownership + admin-create integration suites, OPDS all=1 test
now passes an explicit page size (shared-DB growth made the fresh
books fall off page 1), device-cap helper switched to the admin
endpoint.
The guard returned c.JSON(401, ...), whose nil error told echo the
SuccessHandler succeeded — the pipeline then continued into the route
handler, which type-asserted the never-set "user" context value and
panic-recovered as a 500 appended after the 401 body. The status code
happened to survive (first write wins), which is why the pin test
passed, but every ghost request also executed the target handler with
no user — the exact unauthenticated-execution class the guard exists to
prevent.
Return echo.NewHTTPError(401, ...) instead: non-nil aborts the chain
and renders {"message":"user_not_found: ..."}; the pin test now asserts
that shape. Full suite: 219 groups, zero failures.
The JWT middleware validated signature + expiry and fabricated the
request user purely from token claims, so a signature-valid token for a
DELETED user kept full API access until token expiry — observed live on
hardware (a phone session survived the test suite's user-table
recreation and kept serving its true empty sets; open handoff bug 1).
An ensureUserExistsMiddleware already existed but was only wired on the
frontend SSR group (frontend.go) — every /api group used bare
createJWTMiddleware. The existence check now lives INSIDE
createJWTMiddleware's SuccessHandler so every protected group (SSR and
API alike) gets it:
- After claim validation, GetUser by the token's user_id; a missing row
clears the token cookie and answers 401 user_not_found for API calls
(the app's TokenAuthenticator then re-logins with stored credentials)
or redirects to /login?session=invalid for page requests — same
shape-aware split the error handler already uses.
- Cost: one primary-key lookup per authenticated request (~0.2ms local);
instant revocation, no TTL semantics. The redundant SSR-only
middleware is deleted (its redirect behavior is preserved inside the
guard).
The guard exposed two processing-issues tests that were unknowingly
RELYING on ghost semantics: they nested a second setupTestServer whose
bootstrap deleted the first setup's regular user, then asserted 403
from the dead token. De-nested: a createLibraryDirect helper inserts
the library under the live setup's user, no second bootstrap.
New TestDeletedUserTokenRejected pins the contract: token works while
the user exists -> 401 user_not_found after deletion. Full suite: 219
groups, zero failures.
The verify/heal pipeline verified only INBOUND context-bearing writes;
stored-row coherence was never enforced. SaveProgress seeds missing
fields from the existing row, so every percentage-only write (KOReader
low-confidence conversion, KOReader no-pointer payloads, Kobo markup
ReadingSync / analytics / sync-from-server, locator-less queue entries)
silently re-paired its NEW percentage with the PREVIOUS writer's stale
CFI+context — the incoherent row shape that fed the client-side
far-anchor restore floor (and, via the read-side anchor handles, could
even hand a first-party client the OLD position's scroll address).
KOReader's converter already declared the right intent ("storing
percentage only — the row's percentage restores approximately"); the
seeding betrayed it. SaveProgressRequest gains ClearLocator, set by the
percentage-only call sites, and the seeding rule honors it: stale
epubcfi/context/character_offset clear (the upsert assigns columns
directly, so NULL sticks) unless the request carries its own. A
request-carried context survives so the verify/heal pass can still mint
a fresh anchor from it; the HTTP merge path (field-absent PUTs
preserve) and all anchor-bearing writes are untouched. The fixed-layout
branch already cleared locators unconditionally — unchanged.
Consumer audit: percentage — the only thing every sync round-trip
exchanges — is untouched on all paths; KOReader pulls on locator-less
rows already support the nil-CFI shape (builder guards
progress.Epubcfi.Valid); first-party restores go percentage-first
instead of floor-fighting a stale anchor.
Tests: TestProgressWeb_LocatorCoherence pins clear-on-declared,
request-carried-context survival, and merge-path preservation. Full
suite with the change: 218 groups, zero failures.
Closes the remaining integration-suite failures (handoff open bug 3),
line by line:
- Fixture family (the upsert equivalence): tags_filter_test
("Science Fiction" found 1 of 3; "Fantasy" 404'd on zero results),
media_item_isbn_test (normalization read back the first case's ISBN
everywhere; edge cases saw a nil ISBN; pagination collapsed 5
creates onto 1 row), search_unified_test (its 3 books were actually
1 row + two no-op creates) — all POSTed a constant file_path into
one library while CreateMediaItem upserts ON CONFLICT
(library_id, file_path). Every fixture now uses unique per-item
paths, the suite's universal convention elsewhere. TestTagsFilter
confirmed same-family: counts came right with no second bug.
- New TestMediaItemCreateDuplicatePath pins the honest-create
contract from da13898: 201 for the first create; 200 + duplicate
flag + the EXISTING row (never a 201 lie), second payload not
overwritten.
- library_browse_test: the admin subtest browsed the container's
/tmp and required it non-empty — env-coupled (empty /tmp → "was
[]"). Now browses a self-created t.TempDir with two subdirs and a
decoy file, asserting ONLY directories are listed.
- series_integration_test: three stale assertions re-anchored to the
shipped contracts, each with a ground-truth comment: /dashboard/
sections key identity on the collection NAME (BuildSections ID =
CollectionName; the web/app reorder + hidden prefs persist those
strings) — the kebab query_type is the storage key and is not
exposed; /api/series/books is name-ONLY (the app client and the
web SSR page both send name only; library_id is ignored, never
validated); /api/series is optional-scope BY DESIGN (GetDistinct-
Series: narg IS NULL OR library_id = narg aggregates across
libraries when omitted). The former 400-on-missing-library_id
expectations asserted validation that never shipped.
- gofmt rider on three untouched-in-this-arc test files
(dashboard_integration, search, test_helpers).
Full suite with -parallel=1: 217 groups pass, zero failures.
TestProgressWeb_KoboIntegration 500s'd on every markup with a
last-read-place bookmark: the panic-recovered stack was a nil pointer
dereference in LibraryService.ResolveMediaPath (s.db deref), reached
from KoboHandler.convertKoboCFIToStandard during kepub CFI conversion.
Two-layer bug:
1. Test harness (root cause): the router.Config built in
test_helpers_test.go omitted LibraryService, so router/sync.go's
SetLibraryService(cfg.LibraryService) stored a nil
*services.LibraryService in the interface field. Production wiring
(main.go:246) was always correct.
2. Latent trap (hardening): `h.libraryService != nil` at the
convertKoboCFIToStandard call site can never catch a typed nil —
an interface holding a nil *LibraryService is non-nil. ResolveMediaPath
now degrades with a "library service not configured" error instead
of panicking on a nil receiver; callers already handle the error
by skipping CFI conversion.
Harness-only fix, zero production behavior change: the endpoint now
returns a clean 200 with the position saved wherever kepub conversion
is unavailable instead of a panic-recovered 500.
Verified: TestProgressWeb_KoboIntegration passes all three subtests
(401 unauthenticated, ReadingSync→last-read-place percentage
preservation, last-read-place without prior ReadingSync) and on
immediate rerun — previously it failed every run.
The whole-body "should not display Manga Type / Scan Info" assertions
could never pass: every book-detail page for an admin carries the
edit modal, whose form labels ("Manga Type", "Scan Information") and
option texts ("Yes (Right to Left)") share vocabulary with the
metadata display block. This was pre-existing rot — the test never
ran to completion before today because an earlier panic killed the
process first.
Replaced with value-level checks that only the display block could
produce ("yes and right to left", "B&W"): absent for plain ebooks and
unknown-manga-type items, matching the templ guards in
book_detail.templ. Emoji-icon asserts dropped from the RTL/Story-Arc
positive steps — the redesigned comic badge renders text chips
without icons.
Symptom: repeated full-suite runs poisoned the dev database with
leaked rows (duplicate Prefs Scope libraries, colliding device names,
saved-filter name conflicts on rerun) because tests created rows but
never deleted them, and stale-binary runs left rows the new cleanup
could not match.
- test_helpers: library creation now registers t.Cleanup DELETE (the
leak root cause — createTestLibraryWithFolder never cleaned up).
Suite-wide cleanup passes for libraries/collections/devices/
saved-filters run both at setup (pre-pass against legacy residue)
and in t.Cleanup (no per-run residue). Preserve the admin's real
libraries; delete only convention-named test rows.
- setup seeds canonical system collections every run keyed on
query_type (idempotent — heals renames/stale tombstones), moved out
of the preserved-admin branch so it runs for every suite start.
- dashboard_integration: scope-testing library renamed into the
test-row convention; ordering assertions on the restored
"Want to Read" collection preserved.
- filters_test: filter names moved into the convention (rerun-safe —
name-uniqueness 409 no longer trips second runs); five .(string)
type assertions panic-proofed with require.True guards (a panic here
used to kill the whole test process mid-run, which is why a stack of
"never ran" failures surfaced at once later).
- device_test: device names moved into the convention (ILIKE %test%
hit them; duplicate-device-name conflicts gone).
The test helper createDefaultCollectionsForUser carried its own
kebab-case copy of the default system collections — drifted from the
canonical Title-case list the registration seed and
RestoreSystemCollection use. Every suite run against the shared dev DB
re-created a kebab set beside the canonical one (the duplicate sections
on the customize screens); the dev DB is cleaned and the helper now
uses the canonical list.
Two stale expectations corrected while re-running the suite:
TestGetSections_EndToEndFlow asserted the kebab ids (now asserts the
Title-case names — section ids ARE collection names), and
TestGetSections_MissingLibraryID expected a 400 the handler never
implemented — absent library_id is the All Libraries view by design
(now asserts 200 + aggregated sections).
The dashboard's All Libraries view could render sections across every
library but could not persist its own preferences: the PUT required a
UUID library_id, and the prefs GET-without-library_id matched an
arbitrary library's row through a loose NULL OR-clause.
Storage: a dedicated user_dashboard_preferences_global table (one row
per user, user_id PK → users CASCADE) instead of a NULL/sentinel
library_id — chosen over both because (a) the column's FK to libraries
rules out a sentinel UUID without dropping the constraint or minting a
fake library row, and (b) intent becomes structural: only the explicit
global path writes that table, so a buggy or outside-the-API write can
never masquerade as global. System and user collections are
user-scoped, so the order/hidden arrays are library-independent and
valid in this scope.
API: PUT /api/dashboard/preferences binds library_id as *string —
JSON null/absent = global, empty string = 400 (a lost value must stay
loud, not silently clobber the global scope), valid UUID = per-library
(unchanged). GET without library_id now returns the global row instead
of an arbitrary one; the per-library query's dead NULL OR-arm is
removed. GET /sections keeps requiring library_id for per-library
views and aggregates across libraries when absent (unchanged).
Tests: global save/update-dedupe/read round-trip, empty-string 400,
global-vs-per-library independence. Two suite failures are the
handoff's documented pre-existing rot (duplicate system collections in
the dev DB; a sections test expecting a 400 the handler never
implemented) — reproduced and confirmed unrelated to this change.
Live-verified on the dev stack: null save 200 x2 with a single row
updated, empty string 400, global read carries the second save.
The 10k validator cap rejected legitimate long-form notes — a
scholarly reading note with quoted passages and footnotes lands
around 10.1k chars and failed at save with the raw validator error.
Raise the cap to 100,000 on all six annotation text fields in the
media handler request structs:
- CreateMediaNoteRequest / UpdateMediaNoteRequest Content
- CreateMediaHighlightRequest / UpdateMediaHighlightRequest NoteText
- CreateMediaBookmarkRequest / UpdateMediaBookmarkRequest Notes
No other layer changes: the media_notes / media_highlights columns
are unbounded TEXT, and the KOReader + websocket sync paths never
had a length cap, so the REST API now matches the rest of the
system instead of being the strictest gate.
New integration test pins the boundary: 50k and 100k-char notes
return 201, 100,001 chars returns 400.
setupTestServer reuses the shared dev admin (testuser@tests.bookhoard.internal)
instead of re-inserting it, but never reset its password — once any test
mutated the admin's password, every later test in the run failed to log in
with 401s until the database was manually wiped. The seeding step now
resets the password hash to the known test constant on reuse, so full
integration runs are repeatable against an existing database.
Adds cross_library_move_test.go, seven integration tests covering the
duplicate-content features end to end:
- TestCrossLibraryMovePreservesHistory: a book moved between two same-type
libraries keeps its row ID — progress and annotations survive, no
duplicate, no archived ghost, library_type_name stays truthful.
- TestCrossLibraryCopyStaysIndependent: deliberate copies in two libraries
stay independent rows with isolated progress.
- TestCrossLibraryMoveRejectedForTypeMismatch: a reflowable EPUB is not
repointed between manga libraries; a library_type_mismatch processing
issue is recorded instead.
- TestListHiddenMediaItemsMatchesActiveTwin: archived rows expose their
active same-SHA twin (and rows without one report no match).
- TestMergeArchivedItemIntoActiveTwin: merging moves progress and
annotations onto the active copy and removes the archived row.
- TestMergeArchivedItemRejections: non-admin 403, active source, hash
mismatch, missing target_id, self-merge, and archived target all fail
closed.
- TestLibraryTypeNameTriggerOnLibraryChange: the UPDATE trigger refreshes
library_type_name when a row changes libraries.
Tests create their own temp-dir library folders (host runs don't have
/app/uploads), use unique device identifiers (device_identifier is
UNIQUE and leftover rows broke reruns), and clean up via an explicit
defer that runs while the pool is still open — a t.Cleanup registered
for the same purpose silently no-ops because it executes after
setup.Close has closed the pool.
The device catalog flattened every visible library into one list, so
duplicate copies of the same book each got an entry and search had no
library context. The root feed is now a standard OPDS 1.1 navigation feed
that mirrors the web UI's library model:
- Root /catalog: an "All Books" entry first (the previous flat cross-
library behavior, also still served at ?all=1 for clients that want
the single flat list), followed by one folder entry per visible
library with live book counts from GetVisibleLibraryMediaCounts.
- New GET /library/:libraryId/catalog: acquisition feed scoped to one
visible library (403 when the device owner cannot see it), paginated,
with an up-link back to the root.
- Scoped search: each library feed's rel="search" OpenSearch template
pins &library_id=<id>, so opening search from inside a library folder
searches only that library — clients substitute only {searchTerms},
so no client-side changes are required. Root search stays global.
- Global search entries now carry the owning library as a category (and
as a fallback summary when the book has no description), so duplicate
copies are distinguishable in unscoped result lists.
The acquisition entry builder is extracted into addAcquisitionEntries and
shared by the all-books and per-library feeds. bookhoard.koplugin needs
no changes: it only registers the root URL, and KOReader's stock OPDS
client renders navigation feeds natively.
Tests: TestOPDSLibraryFolders covers the nav-feed shape, flat ?all=1
mode, scoped catalog isolation, scoped/global search behavior, and 403s
for libraries outside the device's visibility. Library names avoid the
word "test" on purpose — setupDeviceTest re-runs setupTestServer's
setup-time cleanup, which deletes every library whose name contains it.
schema.sql is re-executed in full on every application startup, so every
statement in it must be idempotent. Two related problems introduced with the
cross-library move feature, plus their fix:
Problem 1: media_items.library_type_name was only populated by a BEFORE
INSERT trigger. When the scanner repoints an existing row to a different
library (cross-library move detection), the denormalized library_type_name
went stale, mislabeling the item's type for validation and display.
Fix: add a second trigger, set_library_type_name_on_library_change, that
fires BEFORE UPDATE OF library_id and re-runs the same population function.
Problem 2 (outage): the new trigger EXECUTE FUNCTIONs set_library_type_name(),
but the pre-existing idempotency dance drops that function on every startup
before recreating it. On the second and later boots, DROP FUNCTION failed
with SQLSTATE 2BP01 (function still depended on by the trigger created by
the previous boot), aborting the whole schema transaction and crash-looping
the container.
Fix: drop BOTH triggers before dropping the function, and create both after
it. The ordering now survives any number of restarts on any database state.
Adds TestSchemaInitializationIsIdempotent, which replays the production
database.Initialize twice against the same database so this class of
"second startup" regression fails in CI instead of in production. Note the
test uses a dedicated pgxpool: Initialize holds the advisory lock on one
connection while executing the schema on another, which deadlocks against
the shared test pool's MaxConns=1.
Two pre-existing harness breaks found while running the device tests:
- RegisterRoutes dereferences cfg.Settings (auth rate limit) but
setupTestServer never set it — every integration test nil-panicked at
route registration. Wire database.NewSettingsRegistry(queries) the
same way main.go does.
- The cleanup deliberately PRESERVES testuser@tests.bookhoard.internal
(shared dev admin), but setup then blindly re-INSERTed that user, so
every run after the first failed on users_email_key. Reuse the user
when it exists; default collections are created only for a NEW user
(the preserved admin already has its set).
Also documented the offline runner used on slow links: build the test
binary on the host (CGO_ENABLED=0 go test -c) and execute it inside the
bookhoard-tests image against the compose network — no in-container
module downloads, no image rebuild.
App reinstalls that preserve data (Android Studio installDebug over an
existing install) re-register with the same device_identifier, but the
devices row from the previous install still exists — device_identifier
is UNIQUE, so ApproveDevice's blind INSERT failed with a unique
violation and returned 500 'failed to create device' (reproduced via
curl: second approve with the same identifier = instant 500; the ~98s
in the original report was app-side retry/polling, not server wait).
ApproveDevice is now idempotent: look the device up by identifier
first; a row owned by the approving user gets its auth token rotated
via UpdateDeviceAuthToken (row id unchanged, so synced highlights/
bookmarks/progress anchored to it stay valid; fresh install = fresh
credentials, old token invalidated); a row owned by another user gets
409; unknown identifiers INSERT as before, with the 23505 race falling
through to the rotate path. DB failures are logged (they were silent).
Also guard the in-memory pendingRegistrations map with a mutex —
register/approve/reject/status/list all touch it from HTTP goroutines,
and a racing write is a Go runtime fatal, not an error. The approver's
credential publication and the status poller's approved-branch snapshot
now run under the lock so the token can never be read half-written.
Regression test: TestApproveDeviceReapprovalRotatesToken — register →
approve → re-register same identifier → approve (must be 200) → token
rotated, exactly one devices row, row carries the new token.
The bookhoard container crashed with 'panic: Failed to create file
watcher: too many open files' (media_scanner.go) after running for a few
hours, preceded by floods of 'no space left on device' from watcher.Add.
Root cause: every scan job called NewMediaScanner(), which eagerly
created an fsnotify watcher. SetFolders() then walked the entire library
tree and registered one inotify watch per directory (~3,000+ across the
libraries), and ScanFolders() registered them again during its walk. The
worker never called scanner.Close() on these ephemeral per-job scanners,
and the worker loop had no recover(), so:
1. Leaked watchers accumulated until the kernel inotify watch cap was
hit (ENOSPC -> 'no space left on device'), then
2. the process fd limit (ulimit -n 1024) was exhausted, causing
fsnotify.NewWatcher() to fail with EMFILE, and
3. NewMediaScanner panicked on that error, taking down the whole
process (exit code 2). With no restart policy the container stayed
down.
The scan jobs run frequently (scan_poll_interval), so the leak built up
within hours. Note this was NOT a disk-space issue; df showed plenty free.
Fix:
- media_scanner.go: NewMediaScanner no longer creates a watcher eagerly
(s.watcher starts nil), which removes the panic site entirely -- there
is nothing to fail at construction. The watcher is created lazily only
when needed.
- media_scanner.go: SetFolders gains a [?1049h[22;0;0t[1;24r(B[m[4l[?7h[?25l[H[2JEvery 2.0s: bool[1;37Hgaruda-ser8: Fri 31 Jul 2026 10:45:41 AM EDT[2;66Hin 0.002s (127)[2;80H
[3dsh: line 1: bool: command not found
[4d[24;1H[?12l[?25h[?1049l[23;0;0t
[?1l> parameter. It creates
and populates a watcher (returning an error instead of panicking) only
when watch=true; otherwise it skips all watcher.Add calls. ScanFolders
guards its watcher.Add with a nil check, and the WatchChanges event
loop exits cleanly when there is no watcher (polling still runs).
- worker.go: the worker() loop now wraps each job in defer/recover() so a
panicking job is recorded as failed and can never kill the process.
- worker.go: the three ephemeral scan handlers (processScanJob,
processSetFoldersJob, processDirectoryScanJob) now defer scanner.Close()
and call SetFolders(..., false), so scan jobs allocate zero watchers and
zero inotify watches. Any pre-existing leak is also bounded by Close().
- handlers/scanner.go: the long-lived watch-mode scanners (StartScanner
and StartWatchModeForLibrary) pass watch=true since they actually read
watcher.Events for live change detection.
- calibre_integration_test.go: updated to the new SetFolders signature
(watch=false, matching one-off scan usage).
Auto-add is fully preserved: new files are still detected by the periodic
poller (startBackupScan), which is independent of fsnotify and unaffected
by these changes. The watch-mode event loop remains as bonus responsiveness
when inotify is available; through Docker bind mounts where inotify is
unreliable, polling is what catches new books.
Complete the annotation sync pipeline across all ingest and serve paths.
Previously, annotations sent inline with KOReader progress pushes were
silently discarded, and no annotations were ever served back to devices.
INGEST (device → server):
KOReader (koreader.go):
- Add processBookAnnotations helper that processes inline highlights,
notes, and bookmarks from every progress push (immediate + checkpoint)
- Highlights get CRE→CFI position conversion before SaveHighlight
- KOReader 'notes' (text + notes) stored as highlights with NoteText
to ensure correct round-trip classification
- Bookmarks routed through SaveBookmark with device sync data
- Called from both updateProgressForBook and handleCheckpointSync
Kobo (kobo.go):
- Markup handler: annotations and bookmarks route through
AnnotationService (SaveHighlight/SaveBookmark)
- Bookmark handler: same routing with device sync data
- SyncFromServer handler: same routing
- All handlers fall back to direct DB calls when annotationSvc == nil
Web reader (media.go):
- CreateMediaHighlight → SaveHighlight (Source="web", ModifiedAt=now)
- CreateMediaNote → SaveNote (Source="web")
- DeleteMediaHighlight → TombstoneHighlightByID
- DeleteMediaNote → TombstoneNoteByID (was hard delete, now tombstone)
- All fall back to old behavior when annotationSvc == nil
SERVE (server → device):
KOReader GetMetadata (koreader.go):
- Query and serve bookmarks from media_bookmarks table (was missing)
- Serve deleted_highlights and deleted_bookmarks arrays containing
device_sync_data + dedup_key for client-side deletion
- Highlights/notes already served with reverse CFI conversion
Kobo Markup handler (kobo.go):
- Track processed books during sync
- Query tombstones per book, extract bookmark_id from device_sync_data
- Return DeletedAnnotations array in KoboSyncStatus response
Conflict resolution (conflicts.go):
- Enable annotation conflict types in ResolveConflict handler
- Add applyAnnotationResolution dispatching to:
applyHighlightResolution / applyBookmarkResolution / applyNoteResolution
- Each looks up by dedup_key and applies winner's fields
- Allow manual override of auto_resolved conflicts
(changed check from != "unresolved" to == "user_resolved")
Infrastructure:
- AnnotationService field + SetAnnotationService in router Config
- Inject AnnotationService into KOReader, Kobo, Media handlers
- Start tombstone purger goroutine in main.go (24h interval)
- Test helpers: construct AnnotationService in test setup
Tests were deleting the development admin user, causing ON DELETE SET NULL
to cascade and set created_by_admin_id to NULL on all libraries.
- test_helpers: skip deletion of testuser@tests.bookhoard.internal
- sync_integration_test: use test-sync% prefix for isolated test data
The TestGetSeries_SpecialCharactersInName test was failing with a 400
status because the series name 'Series: Book & Other (Vol. 1)' was
interpolated directly into the URL without encoding. The ampersand was
parsed as a query parameter delimiter, corrupting the request.
Use url.QueryEscape() to properly encode the name parameter.
Unit tests:
- series_service_test.go: test continueSeriesRowToMediaItems conversion
with valid fields, null fields, and comprehensive field mapping
- series_test.go: test handler initialization and textToString helper
Integration tests (series_integration_test.go):
- GET /api/series: requires library_id, rejects invalid UUID, returns
empty array for empty library, pagination params, limit clamped to
100, response structure validation, special characters in names
- GET /api/series/books: requires library_id and name, handles
nonexistent series, unauthorized access
- Restore Continue Series system collection
- Dashboard sections include all 5 collections (including continue-series)
Update test helpers:
- Add SeriesHandler to setupTestServer router config
- Add 5th Continue Series collection to createDefaultCollectionsForUser
- Update dashboard integration test for 5 collections
The GetReadingStats handler expects dates in MM-DD-YYYY format (01-02-2006)
but the tests were sending YYYY-MM-DD (2006-01-02), causing 400 errors on
the GetReadingStats_WithCustomDateRange and ReadingStats_FutureDateRange
test cases. Updated both test functions to use the matching format.
Add .epub and .pdf to comics type, add .pdf to manga type, and ensure
avif/tiff/tif extensions are consistently included in manga across all
layers. Update test fixtures to match the canonical extension lists.
TestUnifiedSearch: Search for 'zzzznonexistent' instead of 'test' which
matches leftover test data from other tests. Fixes false 200 instead of 404.
TestWebSocketProgressBroadcast: Update to new progress endpoint
/api/media-items/:id/progress with correct PUT body format matching
ProgressService (percentage, epubcfi). Use book_id instead of
media_item_id to match WebSocket broadcast payload field names.
Adds 30 integration tests across 7 test functions covering all progress
endpoints with real HTTP requests and database verification:
- AuthContexts (8 tests): unauthenticated PUT/GET return 401, regular
user and admin both get 200, invalid UUID returns 400, nonexistent
item returns 200 with empty data.
- MergePreservesFields (2 tests): second PUT with only percentage
preserves epubcfi and chapter from first save via GET verification;
web save preserves koreader character_offset via DB query.
- EnrichmentComputesFields (2 tests): character_offset computed from
percentage when total_characters is set on media item; GET returns
enriched format_group and total_characters.
- ConflictDetection (3 tests): different sources with >1% diff within
5 minutes creates sync_conflicts record; same-source rapid saves
create no conflict; <1% diff creates no conflict.
- KoboIntegration (3 tests): ReadingSync then last-read-place preserves
percentage via DB; standalone last-read-place sets epubcfi/chapter;
unauthenticated returns 401.
- KOReaderIntegration (2 tests): Bearer token auth with proper request
body returns 202 Accepted; unauthenticated returns 401.
- DeleteProgress (2 tests): DELETE clears progress; unauthenticated
returns 401.
- EdgeCases (4 tests): empty body succeeds, 0.0% and 1.0% boundaries,
all fields with full DB verification of each column.
Updates test_helpers to create ProgressService in setupTestServer and
inject into all handlers. Fixes previous tests that used testing.Short()
(which caused all tests to be skipped in the container) and assertions
against wrong JSON format (pgtype serializes as plain values, not
wrapped objects).
Go's new() builtin takes a type and allocates a zero value — it cannot
wrap an expression. All instances of new(someExpression) were compile
errors. Replace each with a local variable assignment and address-of
operator.
Affected files:
- handlers/koreader.go: progress field pointers (Chapter, Page, etc.)
- handlers/kobo.go: pagesRemaining pointer
- handlers/queue.go: uuidPtrToString and timestamptzPtrToString helpers
- router/reader.go: bookmark pageNumber and chapterNumber pointers
- services/media_scanner.go: validation error message pointers
- services/worker.go: StartedAt and CompletedAt timestamps
- sync/offline.go: GetDeviceStatus return pointer
- tests/device_test.go: SyncEnabled and SyncFrequencyMinutes pointers
TestCollectionSearchLibraryFilter's 'invalid library_id' case was
expecting a 200 with empty results, but the search handler correctly
returns 404 when no results are found. The test also consumed the
response body for debug logging then tried to JSON-decode the same
body (causing EOF). Add expectedStatus field to the test struct and
return early when a specific non-200 status is expected.
Three issues fixed in processing_issues_test.go:
- Empty UUID: handler returns 400 (uuid.Parse rejects empty string), not 404.
Fix expectedStatus in both List and Stats validation tests.
- Path traversal: raw '../../' in URL creates extra path segments that don't
match the route. Use url.PathEscape so the string is treated as a single
path parameter, letting the handler reject it with 400.
- SQL injection: raw special characters (semicolons, quotes) caused
httptest.NewRequest to panic. url.PathEscape prevents the panic and the
handler rejects the decoded value via uuid.Parse.
- Remove unsupported 'audiobooks' library type from
TestProcessingIssuesDifferentLibraryTypes (only ebooks/comics/manga exist
in the database schema).
The setupTestServer() helper in test_helpers_test.go was not creating
a ProcessingIssuesHandler and not passing one to the router config,
causing a nil pointer dereference when any processing issues route was
hit during tests. Add handler creation and wire it into routerConfig
to match how cmd/server/main.go does it.
Replace all unhandled resp.Body.Close() calls throughout the test suite:
- Deferred calls: replace 'defer VAR.Body.Close()' with a closure that explicitly
discards the error via 'defer func(Body io.ReadCloser) { _ = Body.Close() }(VAR.Body)'
- Immediate calls: replace 'VAR.Body.Close()' with '_ = VAR.Body.Close()'
Replace all unhandled json.NewDecoder(VAR.Body).Decode(&x) calls with error capture
and require.NoError assertion. Files using httptest.ResponseRecorder (collections_preview,
processing_issues) use 'err :=' declaration; suite-style tests (scanner_integration,
dashboard_integration) use s.T() instead of t.
Replace direct error equality check with errors.Is() in media_scanner_hash_test.
In analytics_test.go, improve defer patterns by capturing resp.Body as a named parameter
to avoid stale references, and add require.NoError() checks on all json.NewDecoder().Decode()
calls that were previously silently ignoring decode errors.
Replace the previous mock/httptest-based conflict tests with
integration tests that exercise the full HTTP stack against a live
test server with a real database. Changes include:
- Add shared test helpers (setupConflictTest, createTestConflict,
makeConflictData) to reduce boilerplate across test files
- Split monolithic TestConflictDetection and TestConflictsBulkOperations
into focused test functions per scenario
- Test conflict detection, bulk resolution (most_recent, highest_progress,
manual strategies), and edge cases (empty IDs, invalid UUIDs,
unauthorized access)
- Verify actual database state after resolution, not just HTTP response
Added comprehensive integration tests for the new processing issues API
endpoints that track EPUB format mismatches in manga/comics libraries.
Test Coverage:
- Authentication & authorization (no auth, invalid auth, non-admin, admin)
- Input validation (malformed UUIDs, path traversal, SQL injection attempts)
- Response structure validation (fields, types, content-type)
- Cross-library isolation (ensures issues don't leak between libraries)
- All library types (ebooks, comics, manga, audiobooks)
- Edge cases and error conditions
Endpoints Tested:
- GET /api/libraries/:id/issues/list - Lists unresolved processing issues
- GET /api/libraries/:id/issues/stats - Returns error/warning/info counts
Test Implementation:
- 522 lines, 9 test functions, 30+ subtests
- Uses setupTestServer() helper for server setup
- Uses setupDeviceTest() helper for library creation
- Follows PROJECT_GUIDELINES.md requirements
- Table-driven tests with t.Run() for comprehensive coverage
- Tests all three user contexts: no user, regular user, admin
This ensures the processing issues feature is properly tested before
integration with the media scanner service.
The test was checking for hexadecimal entity ' but templ actually outputs
the decimal entity ' for apostrophes. This commit updates the assertion to
match the actual HTML output from the templ library.
Add comprehensive integration tests for all 8 comic metadata display steps
on the book detail page, ensuring frontend rendering works correctly with
real database data.
## Test Coverage
### Step Tests (8 individual tests)
1. Reading Direction Badge - Tests RTL, LTR, vertical, and auto-hide behavior
2. Community Rating Display - Validates star rendering and numeric score
3. Comic-Specific Badges - Tests age rating, B&W, and story arc badges
4. Universal Series Info - Tests series count, volume, and imprint display
5. Comic-Specific Metadata - Tests manga type, scan info, alternate series
6. Summary Section - Tests ComicInfo.xml summary rendering
7. Metadata Notes Section - Tests technical notes display
8. Web URL Link - Tests external link rendering with security attributes
### Test Case Scenarios (4 complete scenarios)
1. Japanese Manga - Complete metadata display (RTL + all badges)
2. Western Comic - LTR direction with story arc
3. Webtoon/Manhwa - Vertical reading direction
4. Regular Ebook - No comic metadata (minimal display)
### Authentication Tests (2 tests)
- Anonymous users are denied access (401)
- Regular users can view metadata (same as admins)
### Edge Case Tests (2 tests)
- Minimal Metadata - Only required fields (no optional metadata)
- All Fields Together - Comprehensive metadata display
## Test Infrastructure
- Uses setupTestServer() helper for isolated test environment
- Uses createComicMediaItem() helper for flexible test data creation
- Uses createLibrary() helper with automatic cleanup
- Tests use pgtype types matching production code
- All tests run with admin authentication by default
- Tests check both structure and content in rendered HTML
## Test Details
- 21 total subtests covering all metadata display features
- Tests verify HTML structure, content presence, and proper escaping
- Uses t.Run() for organized test output
- Tests clean up resources automatically with t.Cleanup()
- Checks for proper HTML entity encoding (e.g., apostrophes)
- Validates conditional rendering (hide when values not set)
## Known Issues
- Metadata Notes content validation uses partial string matching to handle
HTML escaping variations
- Reading Direction test checks specific direction strings (RTL/LTR/VERTICAL)
to avoid false positives from emoji appearing elsewhere in the UI
- Community Rating test uses colon ("Community Rating:") to avoid matching
HTML comments
Related: Template implementation commit (562ca53)
Update TestRestoreSystemCollection_ValidNames to use the correct
title case format for system collection names.
The API handler validates these specific collection names:
- "Continue Reading"
- "Recently Added"
- "Recently Read"
- "Not Started"
The test was previously using kebab-case names (e.g., "continue-reading")
which were being rejected by the validation logic with 400 Bad Request.
This aligns the test with the updated collection name format used
throughout the application.
This commit fixes multiple issues in the comic metadata test suite that were causing test failures:
1. UUID Byte-Order Corruption
- Fixed byte-order corruption when converting library IDs
- Previously used [16]byte(uuid.MustParse(libraryID)) which corrupted bytes
- Now parse UUID once and reuse the parsed UUID variable
- Matches pattern used successfully in calibre_integration_test.go
2. Test Isolation
- Each sub-test now creates its own isolated library
- Previously all sub-tests shared one library, causing cross-test pollution
- ListMediaItemsByLibrary returns items from previous tests
- New libraries: "RTL Manga Test Library", "Western Comic Test Library", "Minimal Metadata Test Library"
3. Query Function Selection
- Replaced SearchMediaItems with ListMediaItemsByLibrary
- SearchMediaItems requires search_pattern parameter which was missing
- ListMediaItemsByLibrary is simpler and more appropriate for these tests
4. Explicit Default Values
- MangaType and ReadingDirection now explicitly set to expected defaults
- Database defaults not applied when pgtype fields have Valid: false
- "Comic with minimal metadata" test now sets: MangaType="unknown", ReadingDirection="auto"
5. Library Naming for Cleanup
- All library names now include "Test" for proper cleanup
- Test cleanup deletes libraries with "test" in name (case-insensitive)
- Prevents orphaned libraries from accumulating in database
All tests in TestComicMetadataExtraction now pass:
- CBZ with RTL manga ✓
- CBZ with Western comic ✓
- Comic with minimal metadata ✓
- Fix pgtype.UUID usage in test files by properly converting string UUIDs to pgtype.UUID
- Update numericToFloat to use pgtype.Float8 instead of pgtype.Numeric for DOUBLE PRECISION support
- Fix field name from WebURL to WebUrl to match current schema
These changes align with the recent community_rating type change to DOUBLE PRECISION
and ensure consistent type handling across the codebase.
Phase 6.1 implementation: Unit tests for metadata helper functions.
Test Coverage:
- TestNormalizeMangaType: Verify Manga field normalization to database enum values
(unknown, no, yes, yes_and_right_to_left)
- TestDetermineReadingDirection: Test reading direction computation heuristics
(explicit Manga field, Japanese language, webtoon/manhwa genre tags, Western default)
- TestNormalizeAgeRating: Verify age rating standardization
(Everyone, Teen, Mature, Adult with various input formats)
These tests ensure the helper functions correctly normalize ComicInfo.xml data
before storage in the database.
Relates to: Phase 6.1 unit testing
Update TestCollectionSearchLibraryFilter to check for specific test
books rather than exact counts, making tests resilient to changing
dev database data.
Changes:
- Modified "no filter" test case to check both test books are present
- Enhanced shouldContain to support comma-separated book ID lists
- Added strings import for ID list processing
- Skip exact count check when expectedCount is 0
Rationale:
The library_id filter was working correctly. The test failure was due
to running against a dev database with pre-existing data. When no
library_id filter is provided, the API correctly returns all visible
books across all libraries, not just test-created books.
This validates that the filter works correctly while being resilient
to dynamic dev database content.
Fixes: #test-isolation-library-filter
Implement sidecar-first metadata extraction approach that prioritizes
Calibre metadata.opf files over embedded metadata when available.
Key Features:
- Sidecar-first approach: Check for metadata.opf before extracting embedded
- Full Dublin Core namespace support: Use complete namespace URLs
- Calibre-specific meta tags: Extract series, series_index from <meta> tags
- Graceful degradation: Fall back to embedded metadata on parse failure
- Identifier extraction: Support ISBN and ASIN from Dublin Core identifiers
- Date parsing: Handle ISO 8601 timestamps and simple date formats
Implementation Details:
- Added extractCalibreSidecar() to check for and parse metadata.opf
- Added parseCalibreMetadataOPF() with full Dublin Core namespace handling
- Modified extractMetadata() to try sidecar first, fallback to embedded
- Added CalibreOPFMetadata struct for intermediate parsing
- Cover image support: findSidecarCover() for sidecar metadata
Tests:
- Unit tests for parseCalibreMetadataOPF() with real Calibre file examples
- Integration tests for Calibre library scanning
This allows users with Calibre-managed libraries to import their curated
metadata (series, tags, custom covers) into Bookhoard.
Fixes: #calibre-opf-support
- Change type assertion from []map[string]interface{} to []interface{}
- JSON unmarshal into interface{} creates []interface{}, not typed slices
- Fixes panic: interface conversion error in test
The response["results"] field needs to be asserted as []interface{}
when the parent is unmarshaled into map[string]interface{}.
This matches Go's JSON unmarshaling behavior for interface{} types.
- Update test to unmarshal response object before extracting results array
- API returns {"results": [...], "total": N}, not a bare array
- Fixes "cannot unmarshal object into Go value of type []map" error
- Test now correctly handles the structured autocomplete response
The handleFieldValuesSearch endpoint returns a structured response
with metadata (results array + total count), not a bare array.
This aligns the test with the actual API response format.
John O'Keefe
john-okeefe