Commit Graph
7 Commits
Author SHA1 Message Date
John O'Keefe 45b8542e25 fix(api): honor the request's origin as the writer label — app rows stop wearing "web"
Release / build-and-push (push) Successful in 3m36s
Every web-API write endpoint hardcoded Source:"web", so the Android
app's own pushes landed with last_sync_source/last_modified_source
"web" — the app was indistinguishable from the web SPA in progress
conflicts, device_sync_data, and annotation bookkeeping. The app CAN
say who it is (bookmark create already carried origin → origin_source);
the server just ignored it for labeling.

All six write sites (progress PUT, bookmark/note/highlight create+PUT)
now parse an ADDITIVE optional "origin" key, normalized through one
gate: "android"/"ios" pass through, anything absent/unknown stays
"web" (the label keys conflict claims and device_sync_data — free-form
client strings would pollute them). Precedence on progress: a paired
X-Device-Token still outranks origin. Bookmark origin_source now rides
the same gate. The web SPA sends no origin — its rows are unchanged;
KOReader/Kobo handlers keep their own labels.

Wire discipline: additive request keys only (rule 2), response shapes
untouched, pins changelog widened in-commit, locked by
TestOriginWriterAttribution (android lands, default stays web, unknown
normalizes). Full battery: only the two known pre-existing failures
(TestOPDSLibraryFolders rot, TestWebSocketPingPong — both reproduce
with the change stashed).
2026-10-17 21:44:27 -04:00
John O'Keefe 010c0cb287 test(contracts): whitelist the unified row's two new keys on search rows
The H4 row-completion commit made abridged/audio_metadata emit on the
search wire (the endpoint serializes the row directly) — exactly what
the pin exists to surface. Whitelisted per the additive rule with the
changelog entry.
2026-10-17 16:57:20 -04:00
John O'Keefe 7d698ea0d8 fix(annotations): bookmark and note PUTs land through the sync service (LWW by id)
H3 census bugs #2 and #3: both web PUTs used plain queries that
bypassed the sync-aware layer entirely — no LWW, no source bookkeeping,
and locator fields the client couldn't even send. The highlight PUT was
the only one already routed (via HighlightID); the other two now mirror
it:

- SaveBookmarkRequest gains BookmarkID, SaveNoteRequest gains NoteID:
  id-targeted resolution reads the exact row, verifies ownership,
  LWWs under its stored dedup key, and resurrects tombstones when the
  edit is newer (same doctrine and comment structure as the highlight
  path).
- Fields the PUT omits are coalesced from the stored row before the
  full-column sync update — the legacy web PUT sends only
  title/notes/position, and zeroing cfi/percentage/page on every edit
  would have been data loss. Verified both directions by
  TestH3_BookmarkPUTPreservesLocators (omitted survives, explicit
  lands).
- The bookmark PUT's wire DTO gains the create endpoint's locator keys
  as optional fields (documented additive request keys in
  contract_pins.md); the note PUT keeps its content+position wire.

UpdateMediaBookmark/UpdateMediaNote plain queries are now uncalled
(the generated queries stay until the H4 query sweep).
2026-10-17 15:53:12 -04:00
John O'Keefe ff029f1ad5 fix(annotations): note LWW re-save 500'd — UserID missing from the sync update params
Pin-first batch for H3: freeze web annotation CRUD response shapes
(create 201 / update 200 / delete 204; full sqlc row models) before
the stack consolidation touches anything.

The new pins immediately caught a live server bug the 28 unit tests
could never see (annotations_test.go is pure-unit: keys and compare
logic, no DB): applyNoteLWW built UpdateMediaNoteForSyncParams without
UserID, so the query's WHERE id=$1 AND user_id=$13 compared against
zero — every dedup-hit note re-save (same content+position) returned
500 "update note: no rows in result set" and the update never landed.
The bookmark twin passes UserID and works; the note stack is the clone
that dropped the field.

Fix: pass UserID in the params (one line). Pins go green and become
the regression net; changelog records the new surfaces.
2026-10-17 15:38:47 -04:00
John O'Keefe de9e66636a docs(agents): test-failure triage discipline — suspect the test first, but test edits stay asymmetric
2026-10-17 lessons from the hardening session: most test/bruno failures
were harness/schema/environment drift (drifted body keys, truncated
payloads, container-only paths), found by triaging the test before the
server. Codified as: triage order (test schema -> environment -> cheap
probe), with the guard against "writing tests to work" — test/contract
edits are asymmetric with code fixes: only harness plumbing fixes,
additive pin widenings with a changelog entry, or deletions of tests of
removed behavior. Pins were green at a known commit; git history
arbitrates stale-vs-changed. A right-looking test that fails is prima
facie a server bug: stop and flag.

contract_pins.md rule 4: narrowing/removing a pinned key requires the
user's explicit sign-off — a pin freezing buggy behavior is a ledger,
not a license.
2026-10-17 12:39:59 -04:00
John O'Keefe 28bd323505 refactor(progress): SaveProgressRequest becomes the canonical PUT payload; GET serializes from the row
Batch H1 steps 1-2 of the hardening plan. Wire contracts frozen; the
pinned shapes are unchanged modulo the whitelisted additive GET keys
(docs/contract_pins.md).

1. One canonical request struct. The web PUT binds SaveProgressRequest
   directly (identity/attribution fields are json:"-" so they stay
   server-owned) and copies its server fields once — the hand-rolled DTO
   and the 20-line field-copy block are gone. Parse set for existing
   clients is IDENTICAL to the old DTO (verified in web/src: the ebook
   reader sends percentage/current_page/total_pages/reading_mode/epubcfi/
   context_text/chapter/zoom_level — reader.ts:3113-3143; the audiobook
   reader sends percentage/chapter/audio_offset_seconds/elapsed_seconds/
   reading_mode — audio.ts:518-520).

2. GET /api/media-items/:id/progress now serializes through one
   row-struct-driven builder instead of a hand-map, so a row column
   cannot be silently forgotten again (the audio_offset_seconds
   omission). Existing keys are verbatim; the row already carried
   audio_offset_seconds/reading_mode/viewport_x-y/zoom_level/
   scroll_position_x-y/panel_number and they surface as WHITELISTED
   additive keys. The web audiobook reader already reads
   audio_offset_seconds (audio.ts:219) — with the key absent,
   durationless books resumed at chapter start (offset always 0); they
   now resume correctly. The conditional context_text bolt-on and the
   server-derived css_selector/anchor_href block are unchanged —
   restore precedence stays percentage-first, never re-derived.
2026-10-16 17:22:32 -04:00
John O'Keefe a2ea34232c test(contracts): pin wire shapes before the canonical-types migrations
Batch H0 of the hardening plan: freeze the JSON key SETS of every
load-bearing surface so H1-H7 can only change them additively.

- cmd/server/tests/contract_pins_test.go: 14 tests / ~30 subtests pinning
  progress GET+PUT, the KOReader plugin surfaces (progress push/pull,
  library, bookmarks), the Kobo adapter shapes (initialization, markup,
  sync-from-server), collections detail all four branches + preview,
  series JSON, search rows, dashboard sections, media list BOTH branches
  (raw sqlc rows vs resolved 38-key map), and SSR smoke for
  bookshelf/collections/series/tags.
- docs/contract_pins.md: the pinned-shape inventory + the rule that a
  batch widening a shape must widen the pin and this doc in one commit.

Suite is green on the unmodified tree at b4ae423. KOReader/Kobo/progress
pins are the hard freeze (the plugin is an external client).
2026-10-16 16:02:14 -04:00