From 72233e1c6a18e3e3c2fa64f658ece4f1e95b9e41 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Fri, 19 Jun 2026 11:00:40 +0800 Subject: [PATCH] feat: sync reading status across devices and with KOReader (#4634) (#4656) * docs(sync): design spec for syncing reading status (#4634) Field-level LWW for reading_status (dedicated reading_status_updated_at), a new 'abandoned' status in the Readest UI, and a koplugin bridge to KOReader's native summary.status (whole-library apply + capture). Co-Authored-By: Claude Opus 4.8 (1M context) * docs(sync): implementation plan for syncing reading status (#4634) Bite-sized TDD tasks across 3 parts: A) cloud field-level LWW (reading_status_updated_at on server upsert + client pull-merge), B) 'abandoned'/On-hold status in the Readest UI, C) koplugin bridge to KOReader summary.status (mapping + reconcile + whole-library apply/capture). Co-Authored-By: Claude Opus 4.8 (1M context) * feat(sync): add reading_status_updated_at for field-level status LWW (#4634) * feat(sync): stamp readingStatusUpdatedAt on status change in updateBookProgress Co-Authored-By: Claude Opus 4.8 (1M context) * feat(sync): stamp status timestamp on explicit library status edits * feat(sync): resolve reading status by its own timestamp in client pull-merge * feat(sync): resolve reading status by its own timestamp in server upsert (#4634) * fix(sync): tighten reading-status merge typing + strengthen test (A5 review) Replace as-unknown-as double-casts at read sites with typed locals (clientBook/serverBook); retain a single as-unknown-as only at the server-wins construction site where the static type is too narrow. Strengthen test 3 to assert both fields with toEqual. Co-Authored-By: Claude Sonnet 4.6 * feat(library): render the 'On hold' (abandoned) status badge * feat(library): add 'Mark as On hold' actions + i18n for abandoned status Co-Authored-By: Claude Sonnet 4.6 * i18n: translate 'On hold' and 'Mark as On hold' for the abandoned status (#4634) Co-Authored-By: Claude Sonnet 4.6 * feat(koplugin): add reading-status mapping + reconcile between Readest and KOReader * feat(koplugin): persist + sync reading_status_updated_at in LibraryStore Co-Authored-By: Claude Sonnet 4.6 * feat(koplugin): bridge reading status to KOReader summary.status on library sync (#4634) * test(sync): cover koplugin v1->v2 migration + tighten status-sync tests (final review) Co-Authored-By: Claude Opus 4.8 (1M context) * docs(sync): redesign KOReader first-sync (decisive-only + bootstrap) (#4634) Co-Authored-By: Claude Opus 4.8 (1M context) * fix(koplugin): safe first-sync of reading status (decisive-only + bootstrap) (#4634) KOReader auto-sets summary.status='reading' on first open, and legacy Readest statuses have reading_status_updated_at=0, so pure timestamp LWW let opening a finished book downgrade it. Restrict sync to deliberate statuses (finished/ complete, abandoned/on-hold, unread->clear); never capture KO 'reading'/'New'. On the unsynced baseline (Readest ts=0) conflicts resolve Readest-authoritative, then stamp now_ms to exit bootstrap into steady-state LWW. reconcile now returns write_ko/write_store flags; statussync captures now_ms once and equalizes both sides (convergent, idempotent, resumable). Co-Authored-By: Claude Opus 4.8 (1M context) * test(koplugin): cover remaining first-sync graph cells + document sort effect (review) Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../2026-06-18-sync-reading-status-design.md | 348 +++++ .../2026-06-18-sync-reading-status-plan.md | 1364 +++++++++++++++++ .../public/locales/ar/translation.json | 2 + .../public/locales/bn/translation.json | 2 + .../public/locales/bo/translation.json | 2 + .../public/locales/de/translation.json | 2 + .../public/locales/el/translation.json | 2 + .../public/locales/en/translation.json | 2 + .../public/locales/es/translation.json | 2 + .../public/locales/fa/translation.json | 2 + .../public/locales/fr/translation.json | 2 + .../public/locales/he/translation.json | 2 + .../public/locales/hi/translation.json | 2 + .../public/locales/hu/translation.json | 2 + .../public/locales/id/translation.json | 2 + .../public/locales/it/translation.json | 2 + .../public/locales/ja/translation.json | 2 + .../public/locales/ko/translation.json | 2 + .../public/locales/ms/translation.json | 2 + .../public/locales/nl/translation.json | 2 + .../public/locales/pl/translation.json | 2 + .../public/locales/pt-BR/translation.json | 2 + .../public/locales/pt/translation.json | 2 + .../public/locales/ro/translation.json | 2 + .../public/locales/ru/translation.json | 2 + .../public/locales/si/translation.json | 2 + .../public/locales/sl/translation.json | 2 + .../public/locales/sv/translation.json | 2 + .../public/locales/ta/translation.json | 2 + .../public/locales/th/translation.json | 2 + .../public/locales/tr/translation.json | 2 + .../public/locales/uk/translation.json | 2 + .../public/locales/uz/translation.json | 2 + .../public/locales/vi/translation.json | 2 + .../public/locales/zh-CN/translation.json | 2 + .../public/locales/zh-TW/translation.json | 2 + .../app/library/book-context-menu.test.ts | 23 +- .../app/library/reading-status-helper.test.ts | 55 + .../app/library/status-badge.test.tsx | 20 + .../pages/api/sync-reading-status.test.ts | 30 + .../src/__tests__/store/library-store.test.ts | 19 + .../src/__tests__/utils/transform.test.ts | 57 +- .../src/app/library/components/Bookshelf.tsx | 5 +- .../app/library/components/BookshelfItem.tsx | 6 + .../library/components/ReadingProgress.tsx | 14 + .../app/library/components/SetStatusAlert.tsx | 6 + .../app/library/components/StatusBadge.tsx | 25 +- .../src/app/library/hooks/useBooksSync.ts | 6 + .../src/app/library/utils/libraryUtils.ts | 41 +- apps/readest-app/src/pages/api/sync.ts | 61 +- apps/readest-app/src/store/libraryStore.ts | 2 + apps/readest-app/src/types/book.ts | 3 +- apps/readest-app/src/types/records.ts | 1 + apps/readest-app/src/utils/transform.ts | 8 + .../readest.koplugin/library/librarystore.lua | 31 +- .../library/librarywidget.lua | 56 +- .../library/readingstatus.lua | 108 ++ apps/readest.koplugin/library/statussync.lua | 45 + apps/readest.koplugin/library/syncbooks.lua | 1 + .../spec/library/librarystore_spec.lua | 118 +- .../spec/library/readingstatus_spec.lua | 203 +++ .../spec/library/statussync_spec.lua | 81 + .../spec/library/syncbooks_spec.lua | 7 + docker/volumes/db/init/schema.sql | 1 + .../015_add_reading_status_updated_at.sql | 11 + 65 files changed, 2786 insertions(+), 38 deletions(-) create mode 100644 apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-design.md create mode 100644 apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-plan.md create mode 100644 apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts create mode 100644 apps/readest-app/src/__tests__/app/library/status-badge.test.tsx create mode 100644 apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts create mode 100644 apps/readest.koplugin/library/readingstatus.lua create mode 100644 apps/readest.koplugin/library/statussync.lua create mode 100644 apps/readest.koplugin/spec/library/readingstatus_spec.lua create mode 100644 apps/readest.koplugin/spec/library/statussync_spec.lua create mode 100644 docker/volumes/db/migrations/015_add_reading_status_updated_at.sql diff --git a/apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-design.md b/apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-design.md new file mode 100644 index 00000000..2b28a2ea --- /dev/null +++ b/apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-design.md @@ -0,0 +1,348 @@ +# Sync Reading Status — Design + +Issue: [#4634](https://github.com/readest/readest/issues/4634) — "FR: Sync reading status" + +## Problem + +A book's reading status (unread / reading / finished) does not reliably sync +across devices, and does not sync with KOReader. The reporter changes a book's +status near the end of reading (~95%) and the change is lost on other devices. + +### Root cause (cross-device cloud sync) + +Reading status already has full sync plumbing: + +- `ReadingStatus` type and `Book.readingStatus` (`src/types/book.ts:19,114`) +- `books.reading_status` column (`docker/volumes/db/init/schema.sql:19`) +- Two-way mapping in `src/utils/transform.ts:97,137` +- The `book` sync category is **on by default** (`src/services/constants.ts`) + +The data travels, but the **merge clobbers it**. The `books` row carries two +independently-edited fields under one `updated_at`: + +- `reading_status` — rare, intentional edits (library context menu / auto-finish) +- a denormalized `progress` — bumped on **every page turn** + (`updateBookProgress` → `book.updatedAt = Date.now()`, `readerStore.ts:399`, + `libraryStore.ts:114`) + +Both the server upsert (`src/pages/api/sync.ts:362-387`) and the client +pull-merge (`src/app/library/hooks/useBooksSync.ts:128-132`) use **whole-row +last-writer-wins** keyed on that single `updated_at`. So a device that is +actively reading (fresh `updated_at`, stale status) overwrites a deliberate +"finished" set on another device. This is exactly the "change status at ~95%" +scenario: the book is being read on more than one device, so progress updates +dominate the timestamp and win the whole row. + +(The progress→books "piggyback", `sync.ts:436-488`, deliberately writes only +`progress` + `updated_at` and leaves `reading_status` intact — partial +protection, but nothing protects against the library's whole-row push.) + +### Root cause (KOReader) + +The `readest.koplugin` already round-trips the `books` table — including +`reading_status` — through its `LibraryStore` SQLite +(`apps/readest.koplugin/library/librarystore.lua:38`, wire mapping at +`syncbooks.lua:150` / `librarystore.lua:747`). The missing bridge is between +the `LibraryStore.reading_status` and KOReader's **native per-book status** +(`summary.status` in the `.sdr` DocSettings sidecar). Pulled status is never +applied to `summary.status`, and KOReader status changes are never captured +back into the store. + +## Goals + +1. A reading-status change on one Readest device reliably propagates to others + and is never clobbered by an orthogonal reading-progress update. +2. Reading status round-trips between Readest and KOReader: marking a book + finished/reading/on-hold in either reflects in the other. +3. Backward compatible: older clients keep working; the change is additive. + +## Non-goals + +- Changing how reading **progress** (position/CFI) syncs. +- A separate sync-category toggle for status (it stays under `book`). +- Real-time push; existing throttle/debounce cadence is fine. + +## Decisions (locked with maintainer) + +- **Field-level LWW** via a dedicated `reading_status_updated_at` timestamp + (additive nullable column). Chosen over "stop bumping updatedAt on page turns" + because only a per-field timestamp preserves **both** status and progress; + the alternative merely relocates the clobber to the denormalized progress bar. +- **Add an `abandoned` status to Readest** so KOReader's `abandoned` ("On hold") + round-trips losslessly, rather than collapsing it. +- **Whole-library apply** in KOReader: pulled status is written to every + matching local book's sidecar (resolved via `LibraryStore.file_path` / + `local_present`), not just the currently-open book. + +## Data model & merge rules + +### Status value set + +`ReadingStatus = 'unread' | 'reading' | 'finished' | 'abandoned'` +(`undefined`/absent = no explicit status, rendered as a plain progress bar). + +### Readest ⇄ KOReader status mapping + +KOReader stores `summary.status ∈ { "reading", "abandoned", "complete" }` +(unopened books have no sidecar and render as "New"; there is no "unread"). + +**Only deliberate statuses sync.** KOReader auto-sets `summary.status = "reading"` +the *first time a book is opened* — that is not a user decision, so treating it +as a syncable status would let merely opening a finished book downgrade it. +Therefore: + +- **Decisive (sync):** Readest `unread`, `finished`, `abandoned`; KOReader + `complete`, `abandoned`. +- **Non-decisive (ignored as a status signal):** Readest `undefined`/`reading` + (Readest never even sets `reading` explicitly); KOReader `reading` (auto) and + `New`/absent. + +Reading *position* still syncs via the separate progress channel — this section +is only about the status badge. + +| Readest | → KOReader `summary.status` | KOReader | → Readest | +| ------------ | --------------------------- | --------------- | -------------- | +| `finished` | `complete` | `complete` | `finished` | +| `abandoned` | `abandoned` ("On hold") | `abandoned` | `abandoned` | +| `unread` | clear (→ "New") | `reading`/`New` | — (no opinion) | +| `undefined` | — (leave sidecar) | | | + +`unread` in Readest means **"not started / reset"**: it intentionally hides the +progress bar *and* any badge (`SHOW_UNREAD_STATUS_BADGE = false`, +`ReadingProgress` renders nothing — decision from `#3103`/`c58e172a5`), and the +reader clears it back to `undefined` the moment the book is opened +(`readerStore.ts:393-394`). Clearing KOReader's status (→ "New") is its faithful +equivalent; KOReader can't tell "deliberately marked not-started" from "never +opened", which is harmless. + +#### First-sync transfer graph (the unsynced baseline) + +On the baseline — a book whose Readest `reading_status_updated_at` is `0`/absent +(status predates this feature, or was pulled before it) — timestamps are not +trustworthy, so conflicts resolve **Readest-authoritative**: + +| Readest ↓ \ KOReader → | `New`/`reading` (auto) | `complete` | `abandoned` | +| ---------------------- | ---------------------- | ----------------------- | ----------------------- | +| `undefined` | — (nothing) | capture → `finished` | capture → `abandoned` | +| `unread` | push → clear KO | Readest wins → clear KO | Readest wins → clear KO | +| `finished` | push → KO `complete` | agree | Readest wins → `complete` | +| `abandoned` | push → KO `abandoned` | Readest wins → `abandoned` | agree | + +The two reported cases fall out of this graph: a `finished`-in-Readest book that +is *opened* in KOReader hits the `finished × reading` cell → **push down to +`complete`, no downgrade**; a `reading`-in-KOReader / `undefined`-in-Readest book +hits `undefined × reading` → **nothing synced**. + +### Field-level last-writer-wins + +`reading_status` is merged by `reading_status_updated_at` (ms), **independent** +of the row's `updated_at`. All other `books` columns keep whole-row LWW on +`updated_at`/`deleted_at` (unchanged). Concretely, when merging two versions of +a books row: + +- pick the base row by the existing whole-row LWW (`updated_at`/`deleted_at`); +- then override `reading_status` + `reading_status_updated_at` with whichever + side has the greater `reading_status_updated_at`. + +This must run on **both** ends (server upsert and client pull-merge) so neither +direction can clobber the field. A missing `reading_status_updated_at` is +treated as `0` (oldest), so legacy rows never beat a real status edit. + +### Timestamp stamping rule + +`reading_status_updated_at` is set to "now" **only when `reading_status` +actually changes** — never on a pure progress update. Sources: + +- Readest explicit edits (`Bookshelf.tsx` status handlers, `SetStatusAlert`). +- Readest auto-status in the reader (`readerStore.ts`: `unread`→cleared on open, + →`finished` at 100%) — stamped inside `updateBookProgress` only when the + status arg differs from the existing value. +- KOReader: a *captured* decisive status (`complete`/`abandoned`) is stamped + with `summary.modified` parsed to day-ms (KOReader's own change date), falling + back to the sync time if absent. (Day-granularity; see Known limitations.) +- Bootstrap exit: the first reconcile of a book that has a decisive status but a + `0`/absent Readest `reading_status_updated_at` stamps it with the sync time, so + every later change resolves by ordinary LWW instead of the Readest-authoritative + baseline rule (see First sync below). + +## Part A — Cloud field-level LWW (Readest web/app) + +**Schema** (`docker/volumes/db/init/schema.sql` + new numbered migration in +`docker/volumes/db/migrations/`, applied to prod Supabase): + +```sql +ALTER TABLE public.books ADD COLUMN IF NOT EXISTS reading_status_updated_at timestamptz NULL; +``` + +Additive, nullable, backward compatible. + +**Types** — `src/types/book.ts`: +- extend `ReadingStatus` with `'abandoned'`; +- add `Book.readingStatusUpdatedAt?: number`; +- add `DBBook.reading_status_updated_at?: string` (`src/types/records.ts`). + +**Transform** — `src/utils/transform.ts`: +- to DB: `reading_status_updated_at: readingStatusUpdatedAt ? new Date(...).toISOString() : null`; +- from DB: `readingStatusUpdatedAt: reading_status_updated_at ? Date.parse(...) : undefined`. + +**libraryStore.updateBookProgress** (`src/store/libraryStore.ts`): +- stamp `readingStatusUpdatedAt = Date.now()` iff `readingStatus !== book.readingStatus`; + otherwise carry the existing value through. + +**Explicit status edits** (`src/app/library/components/Bookshelf.tsx` +`updateBooksStatus` + `handleUpdateReadingStatus`): set +`readingStatusUpdatedAt: Date.now()` alongside `readingStatus`/`updatedAt`. + +**Client pull-merge** (`src/app/library/hooks/useBooksSync.ts` `processOldBook`): +after the existing `mergedBook` whole-object LWW, override +`readingStatus`/`readingStatusUpdatedAt` by the greater `readingStatusUpdatedAt`. + +**Server upsert** (`src/pages/api/sync.ts`, `books` branch only): keep whole-row +LWW for the row, but resolve `reading_status` / `reading_status_updated_at` by +the field-level rule. This means a corrected row may need to be written even +when the whole row "loses" (server newer overall, client status newer, or vice +versa). Implement as a books-specific post-step so `book_configs`/`book_notes` +are untouched. + +## Part B — `abandoned` status in the Readest UI + +- `StatusBadge` (`src/app/library/components/StatusBadge.tsx`): `abandoned` is a + **visible badge** (label "On hold"; distinct color, eink-safe per DESIGN.md) — + treated like `finished`, **not** like the intentionally badge-less `unread`. + "On hold" is a state worth surfacing; the user always sees it. +- `ReadingProgress` (`.../ReadingProgress.tsx`): show the badge for `abandoned` + (keep the progress bar — unlike `unread`, an on-hold book has real progress). +- Context menu (`BookshelfItem.tsx`) + menu ids (`libraryUtils.ts`): add a + "Mark as On hold" action; keep "Mark as Finished" / "Mark as Unread" / + "Clear Status". +- `SetStatusAlert.tsx`: add the batch "On hold" button. +- i18n: new keys via the key-as-content flow (`docs/i18n.md`); run extraction. + +Copy note: internal value is `abandoned` (matches KOReader's stored value); +display label "On hold" mirrors KOReader. Final wording is a review item. + +## Part C — KOReader status sync (readest.koplugin) + +**LibraryStore** (`library/librarystore.lua`): add `reading_status_updated_at +INTEGER` to the `books` schema + `BOOK_COLS`, and migrate existing DBs with an +idempotent `ALTER TABLE ... ADD COLUMN` guarded against the +already-exists error (reusing the store's schema-version path if one exists — +to be confirmed in the plan). Carry the field in `row_to_wire` (`syncbooks.lua`) +and `parseSyncRow` (`librarystore.lua`). + +**Status mapping module** (new `library/readingstatus.lua`): pure, unit-tested. +`readest_to_ko` (`finished→complete`, `abandoned→abandoned`, `unread→`clear), +`ko_to_readest` (decisive only: `complete→finished`, `abandoned→abandoned`; +`reading`/`New`/unknown → `nil`), `readest_decisive`, `parse_modified_ms`, and +`reconcile(cloud, ko, now_ms)`. `reconcile` decides the winning decisive status +W (per the transfer graph: only-one-decisive → that side; both-agree → that +status; both-conflict → Readest-authoritative when the Readest ts is `0`, else +LWW) and returns `{ write_ko, write_store, readest_status, ts, ko_status }` so +the caller equalizes both sides to W. The bootstrap stamp uses `now_ms`. + +**Apply + capture, whole-library** — `statussync.reconcileLocalStatuses` runs in +the library sync (after `pullBooks`, before `pushChangedBooks` via the +`before_push` hook; and after pull in pull-only mode), over every row with +`local_present == 1` and a resolvable `file_path`. For each it reads the sidecar +`summary` through injected `deps` (production: `DocSettings:open(file_path)`), +calls `reconcile`, then: +- if `write_ko`: set `summary.status` (+ `summary.modified`), `flush()`, and + `BookList.setBookInfoCacheProperty(file, "status", ...)` — `ko_status` may be + `nil` to clear (→ "New"); +- if `write_store`: `touchBook(hash, { reading_status, reading_status_updated_at })` + so `pushChangedBooks` sends the complete row (pushing through `LibraryStore` + avoids a partial books row that would null out other columns server-side). + +The IO is injected via `deps` (`now_ms`, `open_summary`, `write_status`) so the +walk is unit-testable without DocSettings. + +`statussync.reconcileLocalStatuses(store, deps)` walks `local_present == 1` rows, +calls `reconcile`, then performs `deps.write_status` (when `write_ko`) and +`store:touchBook` (when `write_store`). It equalizes both sides to W, so the next +pass finds no mismatch and stops — convergence holds regardless of which side won. + +#### First sync & failure handling + +- **Bootstrap (Readest ts `0`)** resolves conflicts Readest-authoritative, then + stamps the winning status with `now_ms` so the book leaves bootstrap; every + later change (either side) then resolves by ordinary LWW. So "Readest wins" + applies only to the *initial* reconciliation, not forever — a deliberate + KOReader status set *after* first sync correctly wins over an old Readest one. +- **Idempotent + convergent + per-book.** A failed/partial first sync just leaves + some books un-baselined; the next sync finishes them, and already-baselined + books re-evaluate to no-op. There is **no global "bootstrap done" flag** — the + per-book timestamp is the marker. +- **Non-destructive ordering.** Writes are durable before the next book; a crash + mid-book re-reconciles that book *identically* (no double-apply, no loss). + A decisive status is never downgraded to a non-decisive one in any ordering. + Cloud push is eventually-consistent via the existing `getChangedBooks` + watermark. + +**i18n + tests**: no new user-facing string (status sync is silent). Busted specs +cover the mappings (decisive-only), the full transfer graph, bootstrap vs +steady-state, both reported cases, and convergence — following existing +`*_spec.lua` idioms. + +## Testing strategy + +Per `.claude/rules/test-first.md`, write failing tests first. + +- **Part A unit (vitest)**: a merge helper test proving status survives when the + other side is whole-row-newer due to progress; transform round-trip incl. the + new field and `abandoned`; `updateBookProgress` stamps the status timestamp + only on change. Server merge covered by a focused unit on the books field-level + resolver. +- **Part B**: `StatusBadge` / context-menu tests extended for `abandoned` + (mirror existing `book-context-menu.test.ts`). +- **Part C (busted)**: decisive-only mappings; the full first-sync transfer graph; + bootstrap (Readest-authoritative + stamp-to-exit) vs steady-state LWW; both + reported cases; convergence (no oscillation across two passes). + +## Verification (per `.claude/rules/verification.md`) + +`pnpm test`, `pnpm lint`; `pnpm lint:lua` + `pnpm test:lua` (koplugin changed); +no `src-tauri/` changes expected (skip Rust gates). DB migration applied to the +docker schema and a new numbered migration file. + +## Migration & rollout + +- Additive nullable column; old clients ignore it (treated as `0`). +- New `abandoned` value: older clients that read it fall through their + `finished`/`unread` checks to the default progress-bar branch — no crash, + just no badge. Acceptable. +- koplugin `LibraryStore` schema version bump with an idempotent `ADD COLUMN`. + +## Known limitations / risks + +- **KOReader `reading` is never captured.** It is auto-set on first open, so + treating it as a status would downgrade a finished book. The trade-off: a + *deliberate* "I'm reading this" on KOReader is not reflected as a Readest + status — but Readest renders `reading` and `undefined` identically (a progress + bar), and reading *position* syncs via the progress channel, so nothing + user-visible is lost. +- **Decisive-vs-decisive cross-conflict on the unsynced baseline** — e.g. + `finished` in Readest *and* `abandoned` ("On hold") in KOReader, both set + before they ever synced. Resolves Readest-authoritative (per the chosen + policy); rare, and the user can re-set. After first sync, recency LWW governs. +- **KOReader status timestamp is coarse** — `summary.modified` is day-grained, + so a same-day Readest edit (real ms) beats a same-day KOReader change. Minor. +- **`unread` ⇄ KOReader "New"** — `unread` maps to clearing `summary.status`, + exactly KOReader's "New"; KOReader can't distinguish "marked not-started" from + "never opened" (harmless). On the baseline a Readest `unread` will also clear a + KOReader `complete`/`abandoned` (Readest-authoritative); rare. +- **Whole-library reconcile touches many sidecars on first sync** — bounded to + `local_present` rows; per-book conditional writes + one bootstrap stamp each. +- **First sync re-orders the koplugin Library's "recently read" view.** Each + decisive book's bootstrap stamp bumps `updated_at` (load-bearing: it's what + pushes the stamp to the cloud so the next pull doesn't reset it and re-enter + bootstrap), and the Library sort is `COALESCE(updated_at, last_read_at)`. So + status-stamped books cluster at the top once, after the first sync. One-time, + cosmetic; not engineered around in v1. + +## Implementation phasing + +All three parts land in **one PR** on `feat/sync-reading-status`. Implement and +verify internally in order — A (cloud LWW) → B (Readest `abandoned` UI) → +C (koplugin bridge) — since A is the core fix, B adds the value C needs to be +lossless, and C depends on both. Each step is independently testable even though +they ship together. diff --git a/apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-plan.md b/apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-plan.md new file mode 100644 index 00000000..283923d8 --- /dev/null +++ b/apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-plan.md @@ -0,0 +1,1364 @@ +# Sync Reading Status Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make a book's reading status (unread / reading / finished / abandoned) sync reliably across Readest devices and round-trip with KOReader. + +**Architecture:** Reading status is merged by a dedicated per-field timestamp (`reading_status_updated_at`) on both the server upsert and the client pull-merge, independent of the row's `updated_at`, so a high-frequency progress update can never clobber an intentional status change. A new `abandoned` ("On hold") status is added to Readest so KOReader's status set maps losslessly. The `readest.koplugin` bridges its `LibraryStore.reading_status` to KOReader's native `summary.status` via one pure reconcile function used by both the apply (cloud→KOReader) and capture (KOReader→cloud) directions. + +**Tech Stack:** TypeScript / Next.js 16 (web + Tauri), Supabase Postgres, Zustand, Vitest + jsdom, `@testing-library/react`; LuaJIT (`readest.koplugin`) with busted specs. + +**Spec:** `apps/readest-app/.claude/plans/2026-06-18-sync-reading-status-design.md` + +## Global Constraints + +- **Status value set:** `ReadingStatus = 'unread' | 'reading' | 'finished' | 'abandoned'`; `undefined`/absent = no explicit status (plain progress bar). Copy these strings verbatim. +- **Readest ⇄ KOReader map:** `finished↔complete`, `reading↔reading`, `abandoned↔abandoned`, `unread→`clear `summary.status` (KOReader "New"); KOReader "New"/absent contributes no status (never overrides Readest). +- **Field-level LWW:** merge `reading_status` by `reading_status_updated_at` (ms), independent of `updated_at`; missing timestamp = `0` (oldest). Runs on **both** server upsert and client pull-merge. +- **Stamping rule:** set `reading_status_updated_at = now` **only when the status value actually changes** — never on a pure progress update. +- **`abandoned` is a visible badge** (label "On hold"), treated like `finished` (not like the intentionally badge-less `unread`). +- **TypeScript:** never use `any` (use `unknown`/proper types). Strict mode, ES2022. +- **TDD:** write the failing test first, watch it fail, implement minimally, watch it pass, commit. Frequent commits. +- **DB migration** is additive + nullable; new migration file number follows `014_add_reading_stats.sql` → `015_…`. +- **i18n:** new user-facing strings use the key-as-content `_()` flow; run `pnpm i18n:extract` and fill placeholders. +- **Verification gates:** `pnpm test`, `pnpm lint`; `pnpm lint:lua` + `pnpm test:lua` (koplugin changed). No `src-tauri/` changes (skip Rust gates). +- **Branch:** all work lands on `feat/sync-reading-status` (one PR). Implement A → B → C. + +--- + +## Part A — Cloud field-level LWW + +### Task A1: Types, transform mapping, and DB migration + +**Files:** +- Modify: `apps/readest-app/src/types/book.ts:19` (ReadingStatus), `:114` (Book field) +- Modify: `apps/readest-app/src/types/records.ts:13` (DBBook field) +- Modify: `apps/readest-app/src/utils/transform.ts:66-105` (toDB), `:107-144` (fromDB) +- Modify: `docker/volumes/db/init/schema.sql:19` +- Create: `docker/volumes/db/migrations/015_add_reading_status_updated_at.sql` +- Test: `apps/readest-app/src/__tests__/utils/transform.test.ts` (add a describe block) + +**Interfaces:** +- Produces: `ReadingStatus` now includes `'abandoned'`; `Book.readingStatusUpdatedAt?: number`; `DBBook.reading_status_updated_at?: string`; `transformBookToDB`/`transformBookFromDB` round-trip both new fields. + +- [ ] **Step 1: Write the failing test** — append to `transform.test.ts`: + +```ts +import { transformBookToDB, transformBookFromDB } from '@/utils/transform'; +import type { Book } from '@/types/book'; + +describe('transformBook readingStatus + readingStatusUpdatedAt', () => { + const userId = 'user-1'; + const baseBook: Book = { + hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + createdAt: 1, + updatedAt: 2, + }; + + it('serializes abandoned status + timestamp to ISO in the DB record', () => { + const ts = Date.UTC(2026, 5, 18, 12, 0, 0); + const db = transformBookToDB({ ...baseBook, readingStatus: 'abandoned', readingStatusUpdatedAt: ts }, userId); + expect(db.reading_status).toBe('abandoned'); + expect(db.reading_status_updated_at).toBe(new Date(ts).toISOString()); + }); + + it('leaves reading_status_updated_at null when unset', () => { + const db = transformBookToDB({ ...baseBook, readingStatus: 'finished' }, userId); + expect(db.reading_status_updated_at).toBeNull(); + }); + + it('round-trips abandoned + timestamp back to the client shape', () => { + const ts = Date.UTC(2026, 5, 18, 12, 0, 0); + const db = transformBookToDB({ ...baseBook, readingStatus: 'abandoned', readingStatusUpdatedAt: ts }, userId); + const back = transformBookFromDB(db); + expect(back.readingStatus).toBe('abandoned'); + expect(back.readingStatusUpdatedAt).toBe(ts); + }); + + it('reads undefined readingStatusUpdatedAt when the DB column is null', () => { + const back = transformBookFromDB({ + user_id: userId, book_hash: 'h1', format: 'EPUB', title: 'T', author: 'A', + reading_status: 'finished', reading_status_updated_at: null as unknown as undefined, + created_at: new Date(1).toISOString(), updated_at: new Date(2).toISOString(), + }); + expect(back.readingStatusUpdatedAt).toBeUndefined(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/utils/transform.test.ts` +Expected: FAIL — `reading_status_updated_at` is `undefined`/not mapped (and TS may error on the new fields). + +- [ ] **Step 3: Extend the types** + +In `src/types/book.ts` line 19: + +```ts +export type ReadingStatus = 'unread' | 'reading' | 'finished' | 'abandoned'; +``` + +In `src/types/book.ts`, directly after the `readingStatus?: ReadingStatus;` line (114): + +```ts + readingStatus?: ReadingStatus; + readingStatusUpdatedAt?: number; // ms; bumped only when readingStatus changes +``` + +In `src/types/records.ts`, directly after `reading_status?: string;` (13): + +```ts + reading_status?: string; + reading_status_updated_at?: string; +``` + +- [ ] **Step 4: Map both directions in `transform.ts`** + +In `transformBookToDB`, add `readingStatusUpdatedAt` to the destructure (after `readingStatus,`) and add to the returned object (after the `reading_status: readingStatus,` line): + +```ts + reading_status: readingStatus, + reading_status_updated_at: readingStatusUpdatedAt + ? new Date(readingStatusUpdatedAt).toISOString() + : null, +``` + +In `transformBookFromDB`, add `reading_status_updated_at` to the destructure (after `reading_status,`) and add to the returned object (after the `readingStatus: reading_status as ReadingStatus,` line): + +```ts + readingStatus: reading_status as ReadingStatus, + readingStatusUpdatedAt: reading_status_updated_at + ? new Date(reading_status_updated_at).getTime() + : undefined, +``` + +- [ ] **Step 5: Add the schema column + migration** + +In `docker/volumes/db/init/schema.sql`, add after the `reading_status text NULL,` line (19): + +```sql + reading_status text NULL, + reading_status_updated_at timestamp with time zone NULL, +``` + +Create `docker/volumes/db/migrations/015_add_reading_status_updated_at.sql`: + +```sql +-- Migration 015: Add `reading_status_updated_at` to books +-- +-- Field-level last-writer-wins for reading_status. The books row carries +-- both reading_status (rare, intentional) and a denormalized progress +-- (every page turn) under one updated_at, so whole-row LWW lets progress +-- updates clobber a status change across devices (issue #4634). A dedicated +-- per-field timestamp lets the merge resolve reading_status independently. +-- Additive + nullable; NULL is treated as epoch 0 (oldest) by the merge. + +ALTER TABLE public.books + ADD COLUMN IF NOT EXISTS reading_status_updated_at timestamp with time zone NULL; +``` + +- [ ] **Step 6: Run test to verify it passes** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/utils/transform.test.ts` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add apps/readest-app/src/types/book.ts apps/readest-app/src/types/records.ts \ + apps/readest-app/src/utils/transform.ts apps/readest-app/src/__tests__/utils/transform.test.ts \ + docker/volumes/db/init/schema.sql docker/volumes/db/migrations/015_add_reading_status_updated_at.sql +git commit -m "feat(sync): add reading_status_updated_at for field-level status LWW (#4634)" +``` + +--- + +### Task A2: Stamp the status timestamp only on a real status change + +**Files:** +- Modify: `apps/readest-app/src/store/libraryStore.ts:105-122` (`updateBookProgress`) +- Test: `apps/readest-app/src/__tests__/store/library-store.test.ts` (add to the `updateBookProgress` describe) + +**Interfaces:** +- Consumes: `Book.readingStatusUpdatedAt` (Task A1). +- Produces: `updateBookProgress(hash, progress, readingStatus)` sets `readingStatusUpdatedAt = Date.now()` iff `readingStatus !== book.readingStatus`, else preserves the prior value. + +- [ ] **Step 1: Write the failing test** — add inside `describe('updateBookProgress', …)`: + +```ts + test('stamps readingStatusUpdatedAt when the status changes', () => { + useLibraryStore.getState().setLibrary([makeBook({ hash: 'a', readingStatus: undefined })]); + useLibraryStore.getState().updateBookProgress('a', [100, 100], 'finished'); + const book = useLibraryStore.getState().getBookByHash('a'); + expect(book?.readingStatus).toBe('finished'); + expect(book?.readingStatusUpdatedAt).toBeGreaterThan(0); + }); + + test('does NOT change readingStatusUpdatedAt on a progress-only update', () => { + useLibraryStore.getState().setLibrary([ + makeBook({ hash: 'a', readingStatus: 'reading', readingStatusUpdatedAt: 111 }), + ]); + useLibraryStore.getState().updateBookProgress('a', [50, 100], 'reading'); + const book = useLibraryStore.getState().getBookByHash('a'); + expect(book?.readingStatusUpdatedAt).toBe(111); + }); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/store/library-store.test.ts` +Expected: FAIL — `readingStatusUpdatedAt` is `undefined` (not stamped / not preserved). + +- [ ] **Step 3: Update `updateBookProgress`** + +Replace the `updatedBook` construction in `libraryStore.ts` (105-122): + +```ts + updateBookProgress: (hash, progress, readingStatus) => { + const { library, hashIndex } = get(); + const idx = hashIndex.get(hash); + if (idx === undefined) return; + const book = library[idx]!; + const statusChanged = readingStatus !== book.readingStatus; + const updatedBook: Book = { + ...book, + progress, + readingStatus, + readingStatusUpdatedAt: statusChanged ? Date.now() : book.readingStatusUpdatedAt, + updatedAt: Date.now(), + }; + const newLibrary = library.slice(); + newLibrary[idx] = updatedBook; + set({ + library: newLibrary, + visibleLibrary: newLibrary.filter((b) => !b.deletedAt), + }); + }, +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/store/library-store.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add apps/readest-app/src/store/libraryStore.ts apps/readest-app/src/__tests__/store/library-store.test.ts +git commit -m "feat(sync): stamp readingStatusUpdatedAt on status change in updateBookProgress" +``` + +--- + +### Task A3: `withReadingStatus` helper + wire explicit edits + +**Files:** +- Modify: `apps/readest-app/src/app/library/utils/libraryUtils.ts` (add helper near `getBookContextMenuItemIds`) +- Modify: `apps/readest-app/src/app/library/components/Bookshelf.tsx:545` and `:560` +- Test: `apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts` (or a new `reading-status-helper.test.ts`) + +**Interfaces:** +- Produces: `withReadingStatus(book: Book, status: ReadingStatus | undefined): Book` — returns a copy with `readingStatus`, `readingStatusUpdatedAt`, and `updatedAt` all set to the new status / `Date.now()`. + +- [ ] **Step 1: Write the failing test** — create `apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts`: + +```ts +import { describe, expect, it } from 'vitest'; +import { withReadingStatus } from '@/app/library/utils/libraryUtils'; +import type { Book } from '@/types/book'; + +const book: Book = { + hash: 'h1', format: 'EPUB', title: 'T', author: 'A', + createdAt: 1, updatedAt: 2, readingStatus: undefined, +}; + +describe('withReadingStatus', () => { + it('sets status, stamps readingStatusUpdatedAt = updatedAt, and does not mutate input', () => { + const out = withReadingStatus(book, 'abandoned'); + expect(out.readingStatus).toBe('abandoned'); + expect(out.readingStatusUpdatedAt).toBe(out.updatedAt); + expect(out.readingStatusUpdatedAt).toBeGreaterThan(0); + expect(book.readingStatus).toBeUndefined(); // input untouched + }); + + it('clears the status when undefined is passed but still stamps the timestamp', () => { + const out = withReadingStatus({ ...book, readingStatus: 'finished' }, undefined); + expect(out.readingStatus).toBeUndefined(); + expect(out.readingStatusUpdatedAt).toBe(out.updatedAt); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/reading-status-helper.test.ts` +Expected: FAIL — `withReadingStatus` is not exported. + +- [ ] **Step 3: Add the helper** — in `libraryUtils.ts`, just above `export const getBookContextMenuItemIds` (the `Book` and `ReadingStatus` types are already imported there; if `ReadingStatus` is not imported, add it to the existing `@/types/book` import): + +```ts +/** + * Build a new Book with an explicit reading status. Stamps both `updatedAt` + * (so the library sync picks it up) and `readingStatusUpdatedAt` (so the + * field-level merge resolves status independently of progress). Use this for + * every deliberate status edit so the timestamp is never forgotten. + */ +export const withReadingStatus = (book: Book, status: ReadingStatus | undefined): Book => { + const now = Date.now(); + return { ...book, readingStatus: status, readingStatusUpdatedAt: now, updatedAt: now }; +}; +``` + +- [ ] **Step 4: Wire `Bookshelf.tsx`** — replace the body of `updateBooksStatus`'s push (line 545): + +```ts + booksToUpdate.push(withReadingStatus(book, status)); +``` + +and `handleUpdateReadingStatus` (line 560): + +```ts + const updatedBook = withReadingStatus(book, status); +``` + +Add `withReadingStatus` to the existing `@/app/library/utils/libraryUtils` import in `Bookshelf.tsx`. + +- [ ] **Step 5: Run test + typecheck** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/reading-status-helper.test.ts` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add apps/readest-app/src/app/library/utils/libraryUtils.ts \ + apps/readest-app/src/app/library/components/Bookshelf.tsx \ + apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts +git commit -m "feat(sync): stamp status timestamp on explicit library status edits" +``` + +--- + +### Task A4: Client pull-merge resolves status by its own timestamp + +**Files:** +- Modify: `apps/readest-app/src/app/library/utils/libraryUtils.ts` (add `pickFresherReadingStatus`) +- Modify: `apps/readest-app/src/app/library/hooks/useBooksSync.ts:122-135` (`processOldBook`) +- Test: `apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts` + +**Interfaces:** +- Consumes: `withReadingStatus` (A3). +- Produces: `pickFresherReadingStatus(a, b): { readingStatus?: ReadingStatus; readingStatusUpdatedAt?: number }` — returns the status side with the greater `readingStatusUpdatedAt` (ties → `a`). Used by `processOldBook` after the whole-object LWW. + +- [ ] **Step 1: Write the failing test** — append to `reading-status-helper.test.ts`: + +```ts +import { pickFresherReadingStatus } from '@/app/library/utils/libraryUtils'; + +describe('pickFresherReadingStatus', () => { + it('keeps the status whose timestamp is newer, even if the other object is newer overall', () => { + const local = { readingStatus: 'finished' as const, readingStatusUpdatedAt: 200 }; + const remote = { readingStatus: undefined, readingStatusUpdatedAt: 100 }; + expect(pickFresherReadingStatus(local, remote)).toEqual({ + readingStatus: 'finished', readingStatusUpdatedAt: 200, + }); + }); + + it('treats a missing timestamp as oldest', () => { + const local = { readingStatus: undefined, readingStatusUpdatedAt: undefined }; + const remote = { readingStatus: 'abandoned' as const, readingStatusUpdatedAt: 5 }; + expect(pickFresherReadingStatus(local, remote)).toEqual({ + readingStatus: 'abandoned', readingStatusUpdatedAt: 5, + }); + }); + + it('prefers the first argument on a timestamp tie', () => { + const a = { readingStatus: 'reading' as const, readingStatusUpdatedAt: 50 }; + const b = { readingStatus: 'finished' as const, readingStatusUpdatedAt: 50 }; + expect(pickFresherReadingStatus(a, b).readingStatus).toBe('reading'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/reading-status-helper.test.ts` +Expected: FAIL — `pickFresherReadingStatus` is not exported. + +- [ ] **Step 3: Add the helper** — in `libraryUtils.ts`, below `withReadingStatus`: + +```ts +type ReadingStatusFields = Pick; + +/** + * Field-level last-writer-wins for reading status: return whichever side's + * status was set more recently (ties → `a`). Missing timestamp = epoch 0. + * The book row's `updatedAt` is dominated by page-turn progress, so status + * must be resolved by its own timestamp or progress would clobber it. + */ +export const pickFresherReadingStatus = ( + a: ReadingStatusFields, + b: ReadingStatusFields, +): ReadingStatusFields => { + const at = (x: ReadingStatusFields) => x.readingStatusUpdatedAt ?? 0; + const winner = at(a) >= at(b) ? a : b; + return { readingStatus: winner.readingStatus, readingStatusUpdatedAt: winner.readingStatusUpdatedAt }; +}; +``` + +- [ ] **Step 4: Wire `processOldBook`** — in `useBooksSync.ts`, replace the `mergedBook` return (122-135) so the field-level status override runs after the whole-object LWW. Add `pickFresherReadingStatus` to the `@/app/library/utils/libraryUtils` import (create the import if absent): + +```ts + const processOldBook = async (oldBook: Book) => { + const matchingBook = syncedBooks.find((newBook) => newBook.hash === oldBook.hash); + if (matchingBook) { + if (!matchingBook.deletedAt && matchingBook.uploadedAt && !oldBook.coverDownloadedAt) { + oldBook.coverImageUrl = await appService?.generateCoverImageUrl(oldBook); + } + const mergedBook = + matchingBook.updatedAt >= oldBook.updatedAt + ? { ...oldBook, ...matchingBook, syncedAt: Date.now() } + : { ...matchingBook, ...oldBook, syncedAt: Date.now() }; + // Status is resolved by its own timestamp, independent of the row's + // updatedAt (which page-turn progress dominates) — see #4634. + const status = pickFresherReadingStatus(oldBook, matchingBook); + mergedBook.readingStatus = status.readingStatus; + mergedBook.readingStatusUpdatedAt = status.readingStatusUpdatedAt; + return mergedBook; + } + return oldBook; + }; +``` + +- [ ] **Step 5: Run test + full suite** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/reading-status-helper.test.ts` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add apps/readest-app/src/app/library/utils/libraryUtils.ts \ + apps/readest-app/src/app/library/hooks/useBooksSync.ts \ + apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts +git commit -m "feat(sync): resolve reading status by its own timestamp in client pull-merge" +``` + +--- + +### Task A5: Server upsert resolves status by its own timestamp + +**Files:** +- Modify: `apps/readest-app/src/pages/api/sync.ts` (add `resolveReadingStatusMerge` near `pickWinningPages`; wire the `books` branch of `upsertRecords`) +- Test: `apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts` (new) + +**Interfaces:** +- Consumes: `DBBook` (A1). +- Produces: `resolveReadingStatusMerge(client, server): { reading_status?: string; reading_status_updated_at?: string }` — returns the status fields with the greater `reading_status_updated_at` (ties → client). Exported for tests. + +- [ ] **Step 1: Write the failing test** — create `apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts`: + +```ts +import { describe, expect, it } from 'vitest'; +import { resolveReadingStatusMerge } from '@/pages/api/sync'; + +const iso = (ms: number) => new Date(ms).toISOString(); + +describe('resolveReadingStatusMerge', () => { + it('keeps the client status when its status timestamp is newer', () => { + const out = resolveReadingStatusMerge( + { reading_status: 'finished', reading_status_updated_at: iso(200) }, + { reading_status: 'reading', reading_status_updated_at: iso(100) }, + ); + expect(out).toEqual({ reading_status: 'finished', reading_status_updated_at: iso(200) }); + }); + + it('keeps the server status when its status timestamp is newer', () => { + const out = resolveReadingStatusMerge( + { reading_status: 'reading', reading_status_updated_at: iso(100) }, + { reading_status: 'finished', reading_status_updated_at: iso(300) }, + ); + expect(out).toEqual({ reading_status: 'finished', reading_status_updated_at: iso(300) }); + }); + + it('treats a missing timestamp as oldest (server wins over an unstamped client)', () => { + const out = resolveReadingStatusMerge( + { reading_status: undefined, reading_status_updated_at: undefined }, + { reading_status: 'abandoned', reading_status_updated_at: iso(1) }, + ); + expect(out.reading_status).toBe('abandoned'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/pages/api/sync-reading-status.test.ts` +Expected: FAIL — `resolveReadingStatusMerge` is not exported. + +- [ ] **Step 3: Add the helper** — in `sync.ts`, after `pickWinningPages` (38): + +```ts +/** + * Field-level last-writer-wins for a books row's reading_status: return the + * status fields with the newer reading_status_updated_at (ties → client). NULL + * timestamp = epoch 0. Lets reading_status survive even when the whole row is + * decided the other way by updated_at (which page-turn progress dominates) — + * issue #4634. + */ +export function resolveReadingStatusMerge( + client: Pick, + server: Pick, +): Pick { + const ms = (s?: string | null) => (s ? new Date(s).getTime() : 0); + return ms(client.reading_status_updated_at) >= ms(server.reading_status_updated_at) + ? { reading_status: client.reading_status, reading_status_updated_at: client.reading_status_updated_at } + : { reading_status: server.reading_status, reading_status_updated_at: server.reading_status_updated_at }; +} +``` + +- [ ] **Step 4: Wire the `books` branch of `upsertRecords`** — in `sync.ts`, replace the `else` block that compares `clientIsNewer` (the block at ~369-386) so `books` resolves status independently. Use this exact structure: + +```ts + } else { + const clientUpdatedAt = dbRec.updated_at ? new Date(dbRec.updated_at).getTime() : 0; + const serverUpdatedAt = serverData.updated_at + ? new Date(serverData.updated_at).getTime() + : 0; + const clientDeletedAt = dbRec.deleted_at ? new Date(dbRec.deleted_at).getTime() : 0; + const serverDeletedAt = serverData.deleted_at + ? new Date(serverData.deleted_at).getTime() + : 0; + const clientIsNewer = + clientDeletedAt > serverDeletedAt || clientUpdatedAt > serverUpdatedAt; + + if (table === 'books') { + const status = resolveReadingStatusMerge(dbRec as DBBook, serverData as DBBook); + if (clientIsNewer) { + // Client wins the row; graft the fresher status onto it (server's + // status may be the newer one even though the row is older). + (dbRec as DBBook).reading_status = status.reading_status; + (dbRec as DBBook).reading_status_updated_at = status.reading_status_updated_at; + toUpdate.push(dbRec); + } else { + const sd = serverData as DBBook; + // Only rewrite when the resolved status VALUE differs from the + // server's — a timestamp-only difference on the same value is a + // no-op, and rewriting it would churn updated_at + re-propagate. + const statusChanged = status.reading_status !== sd.reading_status; + if (statusChanged) { + // Server wins the row, but the client's status is newer. Write + // server's row with the fresher status and bump updated_at so + // peers re-pull the status change. + toUpdate.push({ + ...sd, + reading_status: status.reading_status, + reading_status_updated_at: status.reading_status_updated_at, + updated_at: new Date().toISOString(), + } as DBBook); + } else { + batchAuthoritativeRecords.push(serverData); + } + } + } else if (clientIsNewer) { + toUpdate.push(dbRec); + } else { + batchAuthoritativeRecords.push(serverData); + } + } +``` + +- [ ] **Step 5: Run test + full suite** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/pages/api/sync-reading-status.test.ts && pnpm test` +Expected: PASS (and no regressions). + +- [ ] **Step 6: Commit** + +```bash +git add apps/readest-app/src/pages/api/sync.ts apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts +git commit -m "feat(sync): resolve reading status by its own timestamp in server upsert (#4634)" +``` + +--- + +## Part B — `abandoned` status in the Readest UI + +### Task B1: Render the `abandoned` badge + +**Files:** +- Modify: `apps/readest-app/src/app/library/components/StatusBadge.tsx` +- Modify: `apps/readest-app/src/app/library/components/ReadingProgress.tsx:28-46` +- Test: `apps/readest-app/src/__tests__/app/library/status-badge.test.tsx` (new) + +**Interfaces:** +- Consumes: `ReadingStatus` incl. `'abandoned'` (A1). +- Produces: `StatusBadge` renders for `finished | unread | abandoned` (returns `null` otherwise); `ReadingProgress` shows the "On hold" badge for `abandoned`. + +- [ ] **Step 1: Write the failing test** — create `status-badge.test.tsx`: + +```tsx +import { describe, expect, it } from 'vitest'; +import { render } from '@testing-library/react'; +import StatusBadge from '@/app/library/components/StatusBadge'; + +describe('StatusBadge', () => { + it('renders children for the abandoned status', () => { + const { queryByText } = render(On hold); + expect(queryByText('On hold')).not.toBeNull(); + }); + + it('renders nothing for the reading status', () => { + const { container } = render(x); + expect(container.firstChild).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/status-badge.test.tsx` +Expected: FAIL — `abandoned` returns `null` (guard excludes it). + +- [ ] **Step 3: Update `StatusBadge.tsx`** — replace the early return and the class block so `abandoned` is supported. Replace the body from `if (status !== 'finished' && status !== 'unread') return null;` through the closing of the `clsx(...)` call with: + +```tsx + if (status !== 'finished' && status !== 'unread' && status !== 'abandoned') return null; + + return ( + + {children} + + ); +``` + +(Remove the now-unused `const isFinished = …` line.) + +- [ ] **Step 4: Update `ReadingProgress.tsx`** — add an `abandoned` branch directly after the `finished` branch (before the `unread` branch). Show the badge alongside the percentage so the on-hold book keeps its progress: + +```tsx + if (book.readingStatus === 'abandoned') { + return ( +
+ {_('On hold')} + {progressPercentage !== null && !Number.isNaN(progressPercentage) && ( + {progressPercentage}% + )} +
+ ); + } +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/status-badge.test.tsx` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add apps/readest-app/src/app/library/components/StatusBadge.tsx \ + apps/readest-app/src/app/library/components/ReadingProgress.tsx \ + apps/readest-app/src/__tests__/app/library/status-badge.test.tsx +git commit -m "feat(library): render the 'On hold' (abandoned) status badge" +``` + +--- + +### Task B2: Context-menu + batch actions for `abandoned`, plus i18n + +**Files:** +- Modify: `apps/readest-app/src/app/library/utils/libraryUtils.ts` (`BookContextMenuItemId`, `getBookContextMenuItemIds`) +- Modify: `apps/readest-app/src/app/library/components/BookshelfItem.tsx:243-263` +- Modify: `apps/readest-app/src/app/library/components/SetStatusAlert.tsx:24-43` +- Test: `apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts` + +**Interfaces:** +- Consumes: `withReadingStatus` (A3), `'abandoned'` status (A1). +- Produces: `'markAbandoned'` menu id; offered when `readingStatus !== 'abandoned'`; `clearStatus` now also offered for `abandoned`. + +- [ ] **Step 1: Write the failing test** — update existing cases and add an abandoned case in `book-context-menu.test.ts`. Replace the "local downloaded book" and "finished book" expectations and add a new test: + +```ts + it('returns a deterministic order for a local downloaded book', () => { + const book = createBook({ downloadedAt: 1 }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', 'group', 'markFinished', 'markAbandoned', + 'showDetails', 'showInFinder', 'searchGoodreads', 'upload', 'share', 'delete', + ]); + }); + + it('shows markUnread + markAbandoned + clearStatus for a finished book', () => { + const book = createBook({ downloadedAt: 1, readingStatus: 'finished' }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', 'group', 'markUnread', 'markAbandoned', 'clearStatus', + 'showDetails', 'showInFinder', 'searchGoodreads', 'upload', 'share', 'delete', + ]); + }); + + it('hides markAbandoned but offers markFinished + clearStatus for an abandoned book', () => { + const book = createBook({ downloadedAt: 1, readingStatus: 'abandoned' }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', 'group', 'markFinished', 'clearStatus', + 'showDetails', 'showInFinder', 'searchGoodreads', 'upload', 'share', 'delete', + ]); + }); +``` + +(Update the existing "unread book" case to include `'markAbandoned'` after `'markFinished'`.) + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/book-context-menu.test.ts` +Expected: FAIL — `markAbandoned` not produced. + +- [ ] **Step 3: Update `libraryUtils.ts`** — add `| 'markAbandoned'` to the `BookContextMenuItemId` union (after `'markUnread'`), and update `getBookContextMenuItemIds`: + +```ts +export const getBookContextMenuItemIds = (book: Book): BookContextMenuItemId[] => { + const ids: BookContextMenuItemId[] = ['select', 'group']; + ids.push(book.readingStatus === 'finished' ? 'markUnread' : 'markFinished'); + if (book.readingStatus !== 'abandoned') ids.push('markAbandoned'); + // "Clear Status" is offered only when the book has an explicit status set. + if ( + book.readingStatus === 'finished' || + book.readingStatus === 'unread' || + book.readingStatus === 'abandoned' + ) { + ids.push('clearStatus'); + } + ids.push('showDetails', 'showInFinder', 'searchGoodreads'); + if (book.uploadedAt && !book.downloadedAt) ids.push('download'); + if (!book.uploadedAt && book.downloadedAt) ids.push('upload'); + if (book.downloadedAt || book.uploadedAt) ids.push('share'); + ids.push('delete'); + return ids; +}; +``` + +- [ ] **Step 4: Add the menu entry** — in `BookshelfItem.tsx`, after the `markUnread` entry (254-257): + +```ts + markAbandoned: { + text: _('Mark as On hold'), + action: async () => { + handleUpdateReadingStatus(book, 'abandoned'); + }, + }, +``` + +- [ ] **Step 5: Add the batch button** — in `SetStatusAlert.tsx`, insert into `statusButtons` before the "Clear Status" entry: + +```ts + { + label: _('Mark as On hold'), + status: 'abandoned' as ReadingStatus, + className: + 'not-eink:bg-slate-500/15 not-eink:text-slate-600 dark:not-eink:text-slate-300 not-eink:border-slate-500/20 eink-bordered', + }, +``` + +- [ ] **Step 6: Run the context-menu test** + +Run: `cd apps/readest-app && pnpm test -- src/__tests__/app/library/book-context-menu.test.ts` +Expected: PASS. + +- [ ] **Step 7: Extract i18n strings** + +Run: `cd apps/readest-app && pnpm i18n:extract` +Then open the locale files under `src/locales/*/translation.json`, find the new `__STRING_NOT_TRANSLATED__` placeholders for `"On hold"` and `"Mark as On hold"`, and translate them (or invoke the project `/i18n` skill which fills placeholders). English (`en`) is key-as-content, so its value equals the key. + +- [ ] **Step 8: Commit** + +```bash +git add apps/readest-app/src/app/library/utils/libraryUtils.ts \ + apps/readest-app/src/app/library/components/BookshelfItem.tsx \ + apps/readest-app/src/app/library/components/SetStatusAlert.tsx \ + apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts \ + apps/readest-app/src/locales +git commit -m "feat(library): add 'Mark as On hold' actions + i18n for abandoned status" +``` + +--- + +## Part C — KOReader status bridge (readest.koplugin) + +### Task C1: `readingstatus.lua` — mapping + reconcile (pure) + +**Files:** +- Create: `apps/readest.koplugin/library/readingstatus.lua` +- Test: `apps/readest.koplugin/spec/library/readingstatus_spec.lua` (new) + +**Interfaces:** +- Produces: + - `readest_to_ko(status) → "complete"|"reading"|"abandoned"|nil` + - `ko_to_readest(status) → "finished"|"reading"|"abandoned"|nil` + - `parse_modified_ms("YYYY-MM-DD") → ms|nil` + - `reconcile(cloud, ko) → { action = "none"|"apply_to_ko"|"apply_to_store", readest_status, ko_status, ts }` where `cloud = { reading_status, reading_status_updated_at }` (ms) and `ko = { status, ts }` (ko summary.status + ms). + +- [ ] **Step 1: Write the failing spec** — create `spec/library/readingstatus_spec.lua`: + +```lua +-- readingstatus_spec.lua — contract for library/readingstatus.lua +require("spec_helper") +local RS = require("library.readingstatus") + +describe("readingstatus mapping", function() + it("maps Readest -> KOReader", function() + assert.are.equal("complete", RS.readest_to_ko("finished")) + assert.are.equal("reading", RS.readest_to_ko("reading")) + assert.are.equal("abandoned", RS.readest_to_ko("abandoned")) + assert.is_nil(RS.readest_to_ko("unread")) + assert.is_nil(RS.readest_to_ko(nil)) + end) + + it("maps KOReader -> Readest", function() + assert.are.equal("finished", RS.ko_to_readest("complete")) + assert.are.equal("reading", RS.ko_to_readest("reading")) + assert.are.equal("abandoned", RS.ko_to_readest("abandoned")) + assert.is_nil(RS.ko_to_readest(nil)) -- "new"/no status -> no opinion + end) + + it("parses summary.modified to day ms", function() + assert.are.equal(os.time({ year = 2026, month = 6, day = 18, hour = 0, min = 0, sec = 0 }) * 1000, + RS.parse_modified_ms("2026-06-18")) + assert.is_nil(RS.parse_modified_ms(nil)) + assert.is_nil(RS.parse_modified_ms("garbage")) + end) +end) + +describe("readingstatus reconcile", function() + it("returns none when both sides already agree", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 100 }, + { status = "complete", ts = 50 }) + assert.are.equal("none", r.action) + end) + + it("applies cloud to KOReader when cloud status is newer", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 300 }, + { status = "reading", ts = 100 }) + assert.are.equal("apply_to_ko", r.action) + assert.are.equal("complete", r.ko_status) + assert.are.equal("finished", r.readest_status) + assert.are.equal(300, r.ts) + end) + + it("applies KOReader to the store when the sidecar status is newer", function() + local r = RS.reconcile({ reading_status = "reading", reading_status_updated_at = 100 }, + { status = "complete", ts = 300 }) + assert.are.equal("apply_to_store", r.action) + assert.are.equal("finished", r.readest_status) + assert.are.equal(300, r.ts) + end) + + it("never lets a KOReader 'new'/no-status book override an existing Readest status", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 10 }, + { status = nil, ts = 9999 }) + assert.are.equal("apply_to_ko", r.action) -- push cloud status down, KO has no opinion + end) + + it("captures a KOReader status when Readest has none", function() + local r = RS.reconcile({ reading_status = nil, reading_status_updated_at = nil }, + { status = "abandoned", ts = 5 }) + assert.are.equal("apply_to_store", r.action) + assert.are.equal("abandoned", r.readest_status) + end) + + it("converges: after applying the winner to both sides, reconcile is a no-op", function() + local r = RS.reconcile({ reading_status = "reading", reading_status_updated_at = 100 }, + { status = "complete", ts = 300 }) + -- emulate equalization: store now holds the winner, sidecar already had it + local r2 = RS.reconcile({ reading_status = r.readest_status, reading_status_updated_at = r.ts }, + { status = "complete", ts = 300 }) + assert.are.equal("none", r2.action) + end) +end) +``` + +- [ ] **Step 2: Run spec to verify it fails** + +Run: `cd apps/readest-app && pnpm test:lua -- spec/library/readingstatus_spec.lua` (or from repo root `pnpm test:lua`) +Expected: FAIL — module `library.readingstatus` not found. + +- [ ] **Step 3: Implement `library/readingstatus.lua`** + +```lua +-- readingstatus.lua — pure bidirectional mapping + reconcile between +-- Readest's reading_status and KOReader's summary.status. No KOReader globals +-- so it unit-tests cleanly under busted. +local M = {} + +local READEST_TO_KO = { finished = "complete", reading = "reading", abandoned = "abandoned" } +local KO_TO_READEST = { complete = "finished", reading = "reading", abandoned = "abandoned" } + +-- Readest reading_status -> KOReader summary.status (nil = clear / "New"). +function M.readest_to_ko(status) + if status == nil then return nil end + return READEST_TO_KO[status] -- 'unread' -> nil (not in the table) +end + +-- KOReader summary.status -> Readest reading_status (nil = KO has no opinion). +function M.ko_to_readest(status) + if status == nil then return nil end + return KO_TO_READEST[status] +end + +-- "YYYY-MM-DD" -> unix ms at local midnight; nil if unparseable. +function M.parse_modified_ms(s) + if type(s) ~= "string" then return nil end + local y, mo, d = s:match("^(%d%d%d%d)%-(%d%d)%-(%d%d)") + if not y then return nil end + local t = os.time({ year = tonumber(y), month = tonumber(mo), day = tonumber(d), + hour = 0, min = 0, sec = 0 }) + if not t then return nil end + return t * 1000 +end + +-- Decide what (if anything) to write. cloud = { reading_status, +-- reading_status_updated_at(ms) }; ko = { status(ko summary.status), ts(ms) }. +-- Returns { action, readest_status, ko_status, ts }. The caller equalizes both +-- sides to (readest_status, ts) so the next reconcile is a no-op (convergence). +function M.reconcile(cloud, ko) + cloud = cloud or {} + ko = ko or {} + local cloud_status = cloud.reading_status + local ko_readest = M.ko_to_readest(ko.status) -- nil if KO has no explicit status + + if cloud_status == ko_readest then + return { action = "none" } + end + + -- KO has no opinion (new/nil): push the cloud status down if one exists. + if ko_readest == nil then + if cloud_status == nil then return { action = "none" } end + return { + action = "apply_to_ko", + readest_status = cloud_status, + ko_status = M.readest_to_ko(cloud_status), + ts = cloud.reading_status_updated_at or 0, + } + end + + -- Readest has no status but KO does: capture it. + if cloud_status == nil then + return { action = "apply_to_store", readest_status = ko_readest, ts = ko.ts or 0 } + end + + -- Both have differing explicit statuses: newer timestamp wins (tie → cloud). + local cloud_ts = cloud.reading_status_updated_at or 0 + local ko_ts = ko.ts or 0 + if cloud_ts >= ko_ts then + return { + action = "apply_to_ko", + readest_status = cloud_status, + ko_status = M.readest_to_ko(cloud_status), + ts = cloud_ts, + } + end + return { action = "apply_to_store", readest_status = ko_readest, ts = ko_ts } +end + +return M +``` + +- [ ] **Step 4: Run spec to verify it passes** + +Run: `cd apps/readest-app && pnpm test:lua -- spec/library/readingstatus_spec.lua` +Expected: PASS (all cases). + +- [ ] **Step 5: Commit** + +```bash +git add apps/readest.koplugin/library/readingstatus.lua apps/readest.koplugin/spec/library/readingstatus_spec.lua +git commit -m "feat(koplugin): add reading-status mapping + reconcile between Readest and KOReader" +``` + +--- + +### Task C2: `LibraryStore` column + migration + wire `reading_status_updated_at` + +**Files:** +- Modify: `apps/readest.koplugin/library/librarystore.lua` (SCHEMA_SQL, SCHEMA_VERSION, BOOK_COLS, NUMERIC_COLS, `M.new` migration, `parseSyncRow`) +- Modify: `apps/readest.koplugin/library/syncbooks.lua` (`row_to_wire`) +- Test: `apps/readest.koplugin/spec/library/librarystore_spec.lua`, `apps/readest.koplugin/spec/library/syncbooks_spec.lua` + +**Interfaces:** +- Consumes: nothing new. +- Produces: `books.reading_status_updated_at` (INTEGER ms) persisted + round-tripped; `parseSyncRow` reads it from the server wire; `row_to_wire` emits `readingStatusUpdatedAt` (ms). + +- [ ] **Step 1: Write the failing specs** + +In `spec/library/librarystore_spec.lua`, add: + +```lua +describe("reading_status_updated_at", function() + it("round-trips reading_status_updated_at through upsert + read", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h1", title = "T", reading_status = "finished", + reading_status_updated_at = 1750000000000, local_present = 1 }) + local row = store:_getRowRaw("h1") + assert.are.equal("finished", row.reading_status) + assert.are.equal(1750000000000, row.reading_status_updated_at) + store:close() + end) + + it("parseSyncRow reads reading_status_updated_at from the server ISO field", function() + local parsed = LibraryStore.parseSyncRow({ + book_hash = "h2", title = "T", reading_status = "abandoned", + reading_status_updated_at = "2026-06-18T00:00:00+00:00", updated_at = "2026-06-18T00:00:00+00:00", + }) + assert.is_truthy(parsed.reading_status_updated_at) + assert.are.equal("abandoned", parsed.reading_status) + end) +end) +``` + +In `spec/library/syncbooks_spec.lua`, add (using the existing `M._row_to_wire` export): + +```lua +it("emits readingStatusUpdatedAt in the wire payload", function() + local wire = syncbooks._row_to_wire({ hash = "h1", title = "T", + reading_status = "finished", reading_status_updated_at = 1750000000000 }) + assert.are.equal("finished", wire.readingStatus) + assert.are.equal(1750000000000, wire.readingStatusUpdatedAt) +end) +``` + +- [ ] **Step 2: Run specs to verify they fail** + +Run: `cd apps/readest-app && pnpm test:lua` +Expected: FAIL — column/field missing. + +- [ ] **Step 3: Update the schema + version + column lists** in `librarystore.lua`: + +Set `local SCHEMA_VERSION = 2`. In `SCHEMA_SQL`, add after `reading_status TEXT,`: + +```sql + reading_status TEXT, + reading_status_updated_at INTEGER, +``` + +Add `"reading_status_updated_at"` to `BOOK_COLS` (after `"reading_status"`) and `reading_status_updated_at = true` to `NUMERIC_COLS`. + +- [ ] **Step 4: Add the migration in `M.new`** — replace the schema-init lines in `M.new`: + +```lua + self.db = SQ3.open(self.db_path) + local prev = self:getUserVersion() or 0 + self.db:exec(SCHEMA_SQL) + -- v1 -> v2: add reading_status_updated_at to existing DBs. CREATE TABLE + -- IF NOT EXISTS won't add a column, so ALTER it in (pcall guards a DB that + -- somehow already has the column). + if prev >= 1 and prev < 2 then + pcall(function() + self.db:exec("ALTER TABLE books ADD COLUMN reading_status_updated_at INTEGER;") + end) + end + self.db:exec(string.format("PRAGMA user_version = %d;", SCHEMA_VERSION)) +``` + +- [ ] **Step 5: Read the field in `parseSyncRow`** — in `librarystore.lua`, after the `out.reading_status = …` line: + +```lua + out.reading_status = dbRow.readingStatus or dbRow.reading_status + -- ms; server sends it as a timestamptz ISO string (iso_to_ms also passes + -- through a raw number when a caller already supplied ms). + out.reading_status_updated_at = iso_to_ms(dbRow.reading_status_updated_at) + or iso_to_ms(dbRow.readingStatusUpdatedAt) +``` + +- [ ] **Step 6: Emit the field in `row_to_wire`** — in `syncbooks.lua`, add to the `out` table (after `readingStatus = row.reading_status,`): + +```lua + readingStatus = row.reading_status, + readingStatusUpdatedAt = num(row.reading_status_updated_at), +``` + +- [ ] **Step 7: Run specs to verify they pass** + +Run: `cd apps/readest-app && pnpm test:lua` +Expected: PASS. + +- [ ] **Step 8: Commit** + +```bash +git add apps/readest.koplugin/library/librarystore.lua apps/readest.koplugin/library/syncbooks.lua \ + apps/readest.koplugin/spec/library/librarystore_spec.lua apps/readest.koplugin/spec/library/syncbooks_spec.lua +git commit -m "feat(koplugin): persist + sync reading_status_updated_at in LibraryStore" +``` + +--- + +### Task C3: `statussync.lua` — apply + capture, wired into the library sync + +**Files:** +- Create: `apps/readest.koplugin/library/statussync.lua` +- Modify: `apps/readest.koplugin/library/librarywidget.lua` (`runCloudSync`) +- Test: `apps/readest.koplugin/spec/library/statussync_spec.lua` (new) + +**Interfaces:** +- Consumes: `readingstatus.reconcile` (C1), `LibraryStore` rows incl. `reading_status_updated_at`, `file_path`, `local_present` (C2), `touchBook` (existing). +- Produces: `reconcileLocalStatuses(store, deps)` — for each local-present row, reads the sidecar `summary` via `deps.open_summary(file_path)`, runs `reconcile`, and on a winner: writes the sidecar (apply) via `deps.write_status(file_path, ko_status)` and/or `store:touchBook(hash, { reading_status, reading_status_updated_at })` (capture). `deps` is injected so the IO is stubbable in tests; production passes a DocSettings-backed `deps`. + +- [ ] **Step 1: Write the failing spec** — create `spec/library/statussync_spec.lua`: + +```lua +require("spec_helper") +local StatusSync = require("library.statussync") +local LibraryStore = require("library.librarystore") + +local function fake_deps(summaries, writes) + return { + now_ms = function() return 1750000000000 end, + open_summary = function(path) return summaries[path] end, -- {status, modified} + write_status = function(path, ko_status) writes[path] = ko_status end, + } +end + +describe("statussync.reconcileLocalStatuses", function() + it("applies a newer cloud status down to the sidecar", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h1", title = "T", file_path = "/b1.epub", local_present = 1, + reading_status = "finished", reading_status_updated_at = 1760000000000 }) + local summaries = { ["/b1.epub"] = { status = "reading", modified = "2026-01-01" } } + local writes = {} + StatusSync.reconcileLocalStatuses(store, fake_deps(summaries, writes)) + assert.are.equal("complete", writes["/b1.epub"]) + store:close() + end) + + it("captures a newer sidecar status into the store and bumps updated_at", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h2", title = "T", file_path = "/b2.epub", local_present = 1, + reading_status = "reading", reading_status_updated_at = 100 }) + local summaries = { ["/b2.epub"] = { status = "complete", modified = "2026-06-18" } } + StatusSync.reconcileLocalStatuses(store, fake_deps(summaries, {})) + local row = store:_getRowRaw("h2") + assert.are.equal("finished", row.reading_status) + assert.is_truthy(row.updated_at) -- touched => dirty for push + store:close() + end) + + it("skips rows without a local file", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h3", title = "T", uploaded_at = 1, local_present = 0, + reading_status = "finished", reading_status_updated_at = 1 }) + local writes = {} + StatusSync.reconcileLocalStatuses(store, fake_deps({}, writes)) + assert.are.same({}, writes) + store:close() + end) +end) +``` + +- [ ] **Step 2: Run spec to verify it fails** + +Run: `cd apps/readest-app && pnpm test:lua -- spec/library/statussync_spec.lua` +Expected: FAIL — module `library.statussync` not found. + +- [ ] **Step 3: Implement `library/statussync.lua`** + +```lua +-- statussync.lua — bridge LibraryStore.reading_status <-> KOReader's per-book +-- summary.status. The decision is delegated to the pure readingstatus.reconcile; +-- this module only walks local-present rows and performs the chosen IO. The IO +-- is injected via `deps` so it unit-tests without DocSettings; production wires +-- a DocSettings-backed deps in librarywidget. +local readingstatus = require("library.readingstatus") + +local M = {} + +-- deps: { now_ms(), open_summary(file_path) -> {status, modified}|nil, +-- write_status(file_path, ko_status_or_nil) } +function M.reconcileLocalStatuses(store, deps) + if not store or not deps then return 0 end + local changed = 0 + local rows = store:listBooks({}) + for _, row in ipairs(rows) do + if row.local_present == 1 and row.file_path then + local summary = deps.open_summary(row.file_path) or {} + local ko_ts = readingstatus.parse_modified_ms(summary.modified) or deps.now_ms() + local r = readingstatus.reconcile( + { reading_status = row.reading_status, + reading_status_updated_at = row.reading_status_updated_at }, + { status = summary.status, ts = ko_ts }) + if r.action == "apply_to_ko" then + deps.write_status(row.file_path, r.ko_status) -- ko_status may be nil (clear) + changed = changed + 1 + elseif r.action == "apply_to_store" then + store:touchBook(row.hash, { + reading_status = r.readest_status, + reading_status_updated_at = r.ts, + }) + changed = changed + 1 + end + end + end + return changed +end + +return M +``` + +- [ ] **Step 4: Run spec to verify it passes** + +Run: `cd apps/readest-app && pnpm test:lua -- spec/library/statussync_spec.lua` +Expected: PASS. + +- [ ] **Step 5: Wire production IO into `runCloudSync`** — in `librarywidget.lua`, build a DocSettings-backed `deps` and run reconcile after pull / before push. Replace the `runCloudSync` body's `syncbooks.syncBooks(...)` call with: + +```lua +local function runCloudSync(opts, store) + local mode = opts.settings.auto_sync and "both" or "pull" + local DocSettings = require("docsettings") + local BookList = require("ui/widget/booklist") + local statussync = require("library.statussync") + local readingstatus = require("library.readingstatus") + local deps = { + now_ms = function() return os.time() * 1000 end, + open_summary = function(file_path) + local ok, ds = pcall(DocSettings.open, DocSettings, file_path) + if not ok or not ds then return nil end + return ds:readSetting("summary") + end, + write_status = function(file_path, ko_status) + local ok, ds = pcall(DocSettings.open, DocSettings, file_path) + if not ok or not ds then return end + local summary = ds:readSetting("summary") or {} + summary.status = ko_status -- nil clears -> KOReader "New" + summary.modified = os.date("%Y-%m-%d", os.time()) + ds:saveSetting("summary", summary) + ds:flush() + BookList.setBookInfoCacheProperty(file_path, "status", ko_status) + end, + } + local function reconcile() statussync.reconcileLocalStatuses(store, deps) end + + logger.info("ReadestLibrary runCloudSync: mode=" .. mode + .. " auto_sync=" .. tostring(opts.settings.auto_sync)) + + local function done(success, msg, status) + logger.info("ReadestLibrary runCloudSync[" .. mode .. "] done: success=" + .. tostring(success) .. " msg=" .. tostring(msg) .. " status=" .. tostring(status)) + M.refresh() + end + + if mode == "both" then + -- before_push runs after pull, before push: apply pulled statuses to + -- sidecars and capture sidecar changes into the store so they're pushed. + syncbooks.syncBooks({ + sync_auth = opts.sync_auth, sync_path = opts.sync_path, + settings = opts.settings, store = store, + }, "both", done, reconcile) + else + syncbooks.syncBooks({ + sync_auth = opts.sync_auth, sync_path = opts.sync_path, + settings = opts.settings, store = store, + }, "pull", function(success, msg, status) + reconcile() -- apply cloud statuses to sidecars even when auto_sync is off + done(success, msg, status) + end) + end +end +``` + +(Keep the surrounding `M.open` scheduling unchanged — only `runCloudSync` changes.) + +- [ ] **Step 6: Run the koplugin lint + tests** + +Run: `cd apps/readest-app && pnpm lint:lua && pnpm test:lua` +Expected: PASS. + +- [ ] **Step 7: Manual verification (live KOReader — not unit-testable IO)** + +1. Install the koplugin on a device with the Readest Library configured + `auto_sync` on. +2. On a Readest device, mark a synced book **Finished**; let it sync. +3. In KOReader, open the Readest Library (triggers `runCloudSync`); confirm the book now shows **Finished** in KOReader (file browser status). +4. In KOReader, set a book to **On hold**; reopen the Readest Library; confirm Readest shows the **On hold** badge after its next library sync. +5. Reopen the Library twice with no changes; confirm no oscillation (status stays put). + +- [ ] **Step 8: Commit** + +```bash +git add apps/readest.koplugin/library/statussync.lua \ + apps/readest.koplugin/library/librarywidget.lua \ + apps/readest.koplugin/spec/library/statussync_spec.lua +git commit -m "feat(koplugin): bridge reading status to KOReader summary.status on library sync (#4634)" +``` + +--- + +## Final verification + +- [ ] `cd apps/readest-app && pnpm test` — all unit tests pass. +- [ ] `cd apps/readest-app && pnpm lint` — Biome + tsgo clean. +- [ ] `cd apps/readest-app && pnpm lint:lua && pnpm test:lua` — koplugin clean. +- [ ] Confirm no `src-tauri/` files changed (`git diff --name-only origin/main -- src-tauri` empty) → Rust gates skipped. +- [ ] Re-read `docs/superpowers`/spec mapping table against `readingstatus.lua` to confirm the four mappings match. + +## Spec coverage map + +- Root-cause clobber fix → Tasks A2 (stamp), A4 (client merge), A5 (server merge). +- Schema/migration → A1 (web), C2 (koplugin store). +- `abandoned` status + UI → A1 (type), B1 (badge), B2 (menu/batch/i18n). +- Readest⇄KOReader mapping + reconcile/convergence → C1. +- Whole-library apply + capture via `file_path`/`local_present` → C3. +- Known limitations (unread→"New", day-granularity) → encoded in `readingstatus.lua` mapping + reconcile (C1) and documented in the spec. + +--- + +## Addendum (first-sync redesign — supersedes the C1/C3 reconcile above) + +After review, the Part C reconcile was redesigned to handle the never-synced +"first sync" between the two libraries safely. See the design spec's updated +"Readest ⇄ KOReader status mapping", "First-sync transfer graph", and +"First sync & failure handling" sections. Key changes vs the original C1/C3: + +- **Decisive-only mapping.** `ko_to_readest` returns a status ONLY for + `complete`→finished and `abandoned`→abandoned; KOReader `reading` (auto-set on + open), `New`, and unknown → `nil` (no opinion, never captured). Readest + `reading`/`undefined` are non-decisive. New helper `readest_decisive`. +- **`reconcile(cloud, ko, now_ms)`** returns + `{ write_ko, write_store, readest_status, ts, ko_status }` (replacing the old + `action` field). It picks the winning decisive status W: only-one-decisive → + that side; both-agree → that status; both-conflict → Readest-authoritative when + `cloud.reading_status_updated_at == 0` (bootstrap), else recency LWW. +- **Bootstrap exit:** when the Readest ts is `0`, the resolved status is stamped + with `now_ms` (via `write_store`) so subsequent syncs use steady-state LWW. +- **`statussync.reconcileLocalStatuses`** captures `now_ms` once, passes it to + `reconcile`, and drives on `write_ko`/`write_store` instead of `action`. +- **Tests** rewritten to cover the decisive-only mappings, the full transfer + graph, bootstrap vs steady, both reported cases, and convergence. + +No web/TS changes — Parts A and B are unaffected. koplugin-only. diff --git a/apps/readest-app/public/locales/ar/translation.json b/apps/readest-app/public/locales/ar/translation.json index 035dd6f4..6cfc2a1f 100644 --- a/apps/readest-app/public/locales/ar/translation.json +++ b/apps/readest-app/public/locales/ar/translation.json @@ -1,4 +1,6 @@ { + "On hold": "معلق", + "Mark as On hold": "تحديد كمعلق", "(detected)": "(تم التعرف)", "About Readest": "حول ريديست", "Add your notes here...": "أضف ملاحظاتك هنا...", diff --git a/apps/readest-app/public/locales/bn/translation.json b/apps/readest-app/public/locales/bn/translation.json index 008feefc..d7c72fb3 100644 --- a/apps/readest-app/public/locales/bn/translation.json +++ b/apps/readest-app/public/locales/bn/translation.json @@ -1,4 +1,6 @@ { + "On hold": "স্থগিত", + "Mark as On hold": "স্থগিত হিসেবে চিহ্নিত করুন", "Email address": "ইমেইল ঠিকানা", "Your Password": "আপনার পাসওয়ার্ড", "Your email address": "আপনার ইমেইল ঠিকানা", diff --git a/apps/readest-app/public/locales/bo/translation.json b/apps/readest-app/public/locales/bo/translation.json index 8152aafc..bc973b6b 100644 --- a/apps/readest-app/public/locales/bo/translation.json +++ b/apps/readest-app/public/locales/bo/translation.json @@ -1,4 +1,6 @@ { + "On hold": "གློད་བཞག", + "Mark as On hold": "གློད་བཞག་ཏུ་རྟགས་རྒྱག", "(detected)": "(བརྟག་དཔྱད་བྱུང་བ།)", "About Readest": "Readest སྐོར།", "Add your notes here...": "འདིར་ཁྱེད་ཀྱི་ཟིན་བྲིས་འདེབས་དང་།...", diff --git a/apps/readest-app/public/locales/de/translation.json b/apps/readest-app/public/locales/de/translation.json index d9237867..75482787 100644 --- a/apps/readest-app/public/locales/de/translation.json +++ b/apps/readest-app/public/locales/de/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Pausiert", + "Mark as On hold": "Als pausiert markieren", "(detected)": "(erkannt)", "About Readest": "Über Readest", "Add your notes here...": "Fügen Sie hier Ihre Notizen hinzu...", diff --git a/apps/readest-app/public/locales/el/translation.json b/apps/readest-app/public/locales/el/translation.json index 498f7ec4..29d046bc 100644 --- a/apps/readest-app/public/locales/el/translation.json +++ b/apps/readest-app/public/locales/el/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Σε αναμονή", + "Mark as On hold": "Σήμανση ως σε αναμονή", "(detected)": "(εντοπίστηκε)", "About Readest": "Σχετικά με το Readest", "Add your notes here...": "Προσθέστε τις σημειώσεις σας εδώ...", diff --git a/apps/readest-app/public/locales/en/translation.json b/apps/readest-app/public/locales/en/translation.json index 91d8b08c..76b08d53 100644 --- a/apps/readest-app/public/locales/en/translation.json +++ b/apps/readest-app/public/locales/en/translation.json @@ -1,4 +1,6 @@ { + "On hold": "On hold", + "Mark as On hold": "Mark as On hold", "LXGW WenKai GB Screen": "LXGW WenKai SC", "LXGW WenKai TC": "LXGW WenKai TC", "Source Han Serif CN": "Source Han Serif", diff --git a/apps/readest-app/public/locales/es/translation.json b/apps/readest-app/public/locales/es/translation.json index b9481723..9eb3ed2a 100644 --- a/apps/readest-app/public/locales/es/translation.json +++ b/apps/readest-app/public/locales/es/translation.json @@ -1,4 +1,6 @@ { + "On hold": "En pausa", + "Mark as On hold": "Marcar como en pausa", "(detected)": "(detectado)", "About Readest": "Acerca de Readest", "Add your notes here...": "Añade tus notas aquí...", diff --git a/apps/readest-app/public/locales/fa/translation.json b/apps/readest-app/public/locales/fa/translation.json index 866dad80..d30ba3b2 100644 --- a/apps/readest-app/public/locales/fa/translation.json +++ b/apps/readest-app/public/locales/fa/translation.json @@ -1,4 +1,6 @@ { + "On hold": "در حال توقف", + "Mark as On hold": "علامت‌گذاری به‌عنوان در حال توقف", "(detected)": "(تشخیص‌داده‌شده)", "About Readest": "درباره‌ی Readest", "Add your notes here...": "یادداشت‌های خود را اینجا اضافه کنید...", diff --git a/apps/readest-app/public/locales/fr/translation.json b/apps/readest-app/public/locales/fr/translation.json index 4ae7cc99..4a1d532c 100644 --- a/apps/readest-app/public/locales/fr/translation.json +++ b/apps/readest-app/public/locales/fr/translation.json @@ -1,4 +1,6 @@ { + "On hold": "En pause", + "Mark as On hold": "Marquer comme en pause", "(detected)": "(détecté)", "About Readest": "À propos de Readest", "Add your notes here...": "Ajoutez vos notes ici...", diff --git a/apps/readest-app/public/locales/he/translation.json b/apps/readest-app/public/locales/he/translation.json index c515709e..6f7ca4a8 100644 --- a/apps/readest-app/public/locales/he/translation.json +++ b/apps/readest-app/public/locales/he/translation.json @@ -1,4 +1,6 @@ { + "On hold": "בהמתנה", + "Mark as On hold": "סמן כבהמתנה", "Email address": "כתובת אימייל", "Your Password": "הסיסמה שלך", "Your email address": "כתובת האימייל שלך", diff --git a/apps/readest-app/public/locales/hi/translation.json b/apps/readest-app/public/locales/hi/translation.json index 914d97da..ec90e03f 100644 --- a/apps/readest-app/public/locales/hi/translation.json +++ b/apps/readest-app/public/locales/hi/translation.json @@ -1,4 +1,6 @@ { + "On hold": "होल्ड पर", + "Mark as On hold": "होल्ड पर के रूप में चिह्नित करें", "(detected)": "(पता लगाया)", "About Readest": "Readest के बारे में", "Add your notes here...": "अपनी टिप्पणियां यहां जोड़ें...", diff --git a/apps/readest-app/public/locales/hu/translation.json b/apps/readest-app/public/locales/hu/translation.json index 72a0bea8..7e0c4854 100644 --- a/apps/readest-app/public/locales/hu/translation.json +++ b/apps/readest-app/public/locales/hu/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Felfüggesztve", + "Mark as On hold": "Megjelölés felfüggesztettként", "Email address": "E-mail cím", "Your Password": "Az Ön jelszava", "Your email address": "Az Ön e-mail címe", diff --git a/apps/readest-app/public/locales/id/translation.json b/apps/readest-app/public/locales/id/translation.json index 56923d17..8540f679 100644 --- a/apps/readest-app/public/locales/id/translation.json +++ b/apps/readest-app/public/locales/id/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Ditunda", + "Mark as On hold": "Tandai sebagai Ditunda", "(detected)": "(terdeteksi)", "About Readest": "Tentang Readest", "Add your notes here...": "Tambahkan catatan Anda di sini...", diff --git a/apps/readest-app/public/locales/it/translation.json b/apps/readest-app/public/locales/it/translation.json index 5cbda803..0799502a 100644 --- a/apps/readest-app/public/locales/it/translation.json +++ b/apps/readest-app/public/locales/it/translation.json @@ -1,4 +1,6 @@ { + "On hold": "In pausa", + "Mark as On hold": "Segna come in pausa", "(detected)": "(rilevato)", "About Readest": "Informazioni su Readest", "Add your notes here...": "Aggiungi qui le tue note...", diff --git a/apps/readest-app/public/locales/ja/translation.json b/apps/readest-app/public/locales/ja/translation.json index 534f88a1..c91f25bc 100644 --- a/apps/readest-app/public/locales/ja/translation.json +++ b/apps/readest-app/public/locales/ja/translation.json @@ -1,4 +1,6 @@ { + "On hold": "保留中", + "Mark as On hold": "保留中にする", "(detected)": "(検出)", "About Readest": "Readestについて", "Add your notes here...": "ここにメモを追加...", diff --git a/apps/readest-app/public/locales/ko/translation.json b/apps/readest-app/public/locales/ko/translation.json index 5da68807..e459deb0 100644 --- a/apps/readest-app/public/locales/ko/translation.json +++ b/apps/readest-app/public/locales/ko/translation.json @@ -1,4 +1,6 @@ { + "On hold": "보류 중", + "Mark as On hold": "보류 중으로 표시", "(detected)": "(감지됨)", "About Readest": "Readest 정보", "Add your notes here...": "여기에 메모를 추가하세요...", diff --git a/apps/readest-app/public/locales/ms/translation.json b/apps/readest-app/public/locales/ms/translation.json index cd16e488..5e736ae5 100644 --- a/apps/readest-app/public/locales/ms/translation.json +++ b/apps/readest-app/public/locales/ms/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Ditangguhkan", + "Mark as On hold": "Tandakan sebagai Ditangguhkan", "Email address": "Alamat e-mel", "Your Password": "Kata Laluan Anda", "Your email address": "Alamat e-mel anda", diff --git a/apps/readest-app/public/locales/nl/translation.json b/apps/readest-app/public/locales/nl/translation.json index d6496621..d2fca9fb 100644 --- a/apps/readest-app/public/locales/nl/translation.json +++ b/apps/readest-app/public/locales/nl/translation.json @@ -1,4 +1,6 @@ { + "On hold": "On hold", + "Mark as On hold": "Markeren als on hold", "Email address": "E-mailadres", "Your Password": "Uw wachtwoord", "Your email address": "Uw e-mailadres", diff --git a/apps/readest-app/public/locales/pl/translation.json b/apps/readest-app/public/locales/pl/translation.json index babe8be9..5a117ab1 100644 --- a/apps/readest-app/public/locales/pl/translation.json +++ b/apps/readest-app/public/locales/pl/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Wstrzymane", + "Mark as On hold": "Oznacz jako wstrzymane", "(detected)": "(wykryto)", "About Readest": "O Readest", "Add your notes here...": "Dodaj swoje notatki tutaj...", diff --git a/apps/readest-app/public/locales/pt-BR/translation.json b/apps/readest-app/public/locales/pt-BR/translation.json index 38e7d290..64e073b6 100644 --- a/apps/readest-app/public/locales/pt-BR/translation.json +++ b/apps/readest-app/public/locales/pt-BR/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Suspenso", + "Mark as On hold": "Marcar como suspenso", "Email address": "Endereço de e-mail", "Your Password": "Sua senha", "Your email address": "Seu endereço de e-mail", diff --git a/apps/readest-app/public/locales/pt/translation.json b/apps/readest-app/public/locales/pt/translation.json index cda9ef09..3c18f93e 100644 --- a/apps/readest-app/public/locales/pt/translation.json +++ b/apps/readest-app/public/locales/pt/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Suspenso", + "Mark as On hold": "Marcar como suspenso", "(detected)": "(detectado)", "About Readest": "Sobre o Readest", "Add your notes here...": "Adicione suas notas aqui...", diff --git a/apps/readest-app/public/locales/ro/translation.json b/apps/readest-app/public/locales/ro/translation.json index 038b57e9..f43bb59f 100644 --- a/apps/readest-app/public/locales/ro/translation.json +++ b/apps/readest-app/public/locales/ro/translation.json @@ -1,4 +1,6 @@ { + "On hold": "În așteptare", + "Mark as On hold": "Marcați ca în așteptare", "Email address": "Adresa de e-mail", "Your Password": "Parola dvs", "Your email address": "Adresa dvs. de e-mail", diff --git a/apps/readest-app/public/locales/ru/translation.json b/apps/readest-app/public/locales/ru/translation.json index fdf975e9..5fc106a1 100644 --- a/apps/readest-app/public/locales/ru/translation.json +++ b/apps/readest-app/public/locales/ru/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Отложено", + "Mark as On hold": "Отметить как отложенное", "(detected)": "(обнаружено)", "About Readest": "О Readest", "Add your notes here...": "Добавьте свои заметки здесь...", diff --git a/apps/readest-app/public/locales/si/translation.json b/apps/readest-app/public/locales/si/translation.json index 4fe67648..09e233e4 100644 --- a/apps/readest-app/public/locales/si/translation.json +++ b/apps/readest-app/public/locales/si/translation.json @@ -1,4 +1,6 @@ { + "On hold": "තාවකාලිකව නතර", + "Mark as On hold": "තාවකාලිකව නතර ලෙස සලකුණු කරන්න", "Email address": "ඊමේල් ලිපිනය", "Your Password": "ඔබේ මුරපදය", "Your email address": "ඔබේ ඊමේල් ලිපිනය", diff --git a/apps/readest-app/public/locales/sl/translation.json b/apps/readest-app/public/locales/sl/translation.json index 9ab9b6b7..89a76df5 100644 --- a/apps/readest-app/public/locales/sl/translation.json +++ b/apps/readest-app/public/locales/sl/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Na čakanju", + "Mark as On hold": "Označi kot na čakanju", "Email address": "E-poštni naslov", "Your Password": "Vaše geslo", "Your email address": "Vaš e-poštni naslov", diff --git a/apps/readest-app/public/locales/sv/translation.json b/apps/readest-app/public/locales/sv/translation.json index 03a7f63c..21b766f3 100644 --- a/apps/readest-app/public/locales/sv/translation.json +++ b/apps/readest-app/public/locales/sv/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Pausad", + "Mark as On hold": "Markera som pausad", "Email address": "E-postadress", "Your Password": "Ditt lösenord", "Your email address": "Din e-postadress", diff --git a/apps/readest-app/public/locales/ta/translation.json b/apps/readest-app/public/locales/ta/translation.json index 5a7af932..c9d824ed 100644 --- a/apps/readest-app/public/locales/ta/translation.json +++ b/apps/readest-app/public/locales/ta/translation.json @@ -1,4 +1,6 @@ { + "On hold": "நிறுத்தி வைக்கப்பட்டது", + "Mark as On hold": "நிறுத்தி வைக்கப்பட்டதாகக் குறிக்கவும்", "Email address": "மின்னஞ்சல் முகவரி", "Your Password": "உங்கள் கடவுச்சொல்", "Your email address": "உங்கள் மின்னஞ்சல் முகவரி", diff --git a/apps/readest-app/public/locales/th/translation.json b/apps/readest-app/public/locales/th/translation.json index 643cb7a0..391d5efe 100644 --- a/apps/readest-app/public/locales/th/translation.json +++ b/apps/readest-app/public/locales/th/translation.json @@ -1,4 +1,6 @@ { + "On hold": "พักอ่าน", + "Mark as On hold": "ทำเครื่องหมายว่าพักอ่าน", "Email address": "อีเมล", "Your Password": "รหัสผ่าน", "Your email address": "อีเมลของคุณ", diff --git a/apps/readest-app/public/locales/tr/translation.json b/apps/readest-app/public/locales/tr/translation.json index 1bf00bcb..9d1f0b89 100644 --- a/apps/readest-app/public/locales/tr/translation.json +++ b/apps/readest-app/public/locales/tr/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Askıda", + "Mark as On hold": "Askıda olarak işaretle", "(detected)": "(algılandı)", "About Readest": "Readest Hakkında", "Add your notes here...": "Notlarınızı buraya ekleyin...", diff --git a/apps/readest-app/public/locales/uk/translation.json b/apps/readest-app/public/locales/uk/translation.json index 32740e01..ffe323bf 100644 --- a/apps/readest-app/public/locales/uk/translation.json +++ b/apps/readest-app/public/locales/uk/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Призупинено", + "Mark as On hold": "Позначити як призупинене", "(detected)": "(виявлено)", "About Readest": "Про Readest", "Add your notes here...": "Додайте свої нотатки сюди...", diff --git a/apps/readest-app/public/locales/uz/translation.json b/apps/readest-app/public/locales/uz/translation.json index 6b3a579e..53b7ff11 100644 --- a/apps/readest-app/public/locales/uz/translation.json +++ b/apps/readest-app/public/locales/uz/translation.json @@ -1,4 +1,6 @@ { + "On hold": "To'xtatilgan", + "Mark as On hold": "To'xtatilgan deb belgilash", "Email address": "Email manzil", "Your Password": "Parolingiz", "Your email address": "Email manzilingiz", diff --git a/apps/readest-app/public/locales/vi/translation.json b/apps/readest-app/public/locales/vi/translation.json index 49a1f131..3df9b093 100644 --- a/apps/readest-app/public/locales/vi/translation.json +++ b/apps/readest-app/public/locales/vi/translation.json @@ -1,4 +1,6 @@ { + "On hold": "Tạm dừng", + "Mark as On hold": "Đánh dấu là tạm dừng", "(detected)": "(đã phát hiện)", "About Readest": "Về Readest", "Add your notes here...": "Thêm ghi chú của bạn vào đây...", diff --git a/apps/readest-app/public/locales/zh-CN/translation.json b/apps/readest-app/public/locales/zh-CN/translation.json index ac0ded08..581778c5 100644 --- a/apps/readest-app/public/locales/zh-CN/translation.json +++ b/apps/readest-app/public/locales/zh-CN/translation.json @@ -1,4 +1,6 @@ { + "On hold": "搁置中", + "Mark as On hold": "标记为搁置", "(detected)": "(检测到)", "About Readest": "关于 Readest", "Add your notes here...": "在这里添加您的笔记...", diff --git a/apps/readest-app/public/locales/zh-TW/translation.json b/apps/readest-app/public/locales/zh-TW/translation.json index d402b75e..406e97da 100644 --- a/apps/readest-app/public/locales/zh-TW/translation.json +++ b/apps/readest-app/public/locales/zh-TW/translation.json @@ -1,4 +1,6 @@ { + "On hold": "擱置中", + "Mark as On hold": "標示為擱置", "(detected)": "(檢測到)", "About Readest": "關於 Readest", "Add your notes here...": "在這裡添加您的筆記...", diff --git a/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts b/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts index 9bffe526..ead3a39b 100644 --- a/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts +++ b/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts @@ -20,6 +20,7 @@ describe('getBookContextMenuItemIds', () => { 'select', 'group', 'markFinished', + 'markAbandoned', 'showDetails', 'showInFinder', 'searchGoodreads', @@ -29,12 +30,13 @@ describe('getBookContextMenuItemIds', () => { ]); }); - it('shows "Mark as Unread" + "Clear Status" for a finished book', () => { + it('shows markUnread + markAbandoned + clearStatus for a finished book', () => { const book = createBook({ downloadedAt: 1, readingStatus: 'finished' }); expect(getBookContextMenuItemIds(book)).toEqual([ 'select', 'group', 'markUnread', + 'markAbandoned', 'clearStatus', 'showDetails', 'showInFinder', @@ -47,6 +49,23 @@ describe('getBookContextMenuItemIds', () => { it('shows "Mark as Finished" + "Clear Status" for an unread book', () => { const book = createBook({ downloadedAt: 1, readingStatus: 'unread' }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', + 'group', + 'markFinished', + 'markAbandoned', + 'clearStatus', + 'showDetails', + 'showInFinder', + 'searchGoodreads', + 'upload', + 'share', + 'delete', + ]); + }); + + it('hides markAbandoned but offers markFinished + clearStatus for an abandoned book', () => { + const book = createBook({ downloadedAt: 1, readingStatus: 'abandoned' }); expect(getBookContextMenuItemIds(book)).toEqual([ 'select', 'group', @@ -67,6 +86,7 @@ describe('getBookContextMenuItemIds', () => { 'select', 'group', 'markFinished', + 'markAbandoned', 'showDetails', 'showInFinder', 'searchGoodreads', @@ -82,6 +102,7 @@ describe('getBookContextMenuItemIds', () => { 'select', 'group', 'markFinished', + 'markAbandoned', 'showDetails', 'showInFinder', 'searchGoodreads', diff --git a/apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts b/apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts new file mode 100644 index 00000000..d549d516 --- /dev/null +++ b/apps/readest-app/src/__tests__/app/library/reading-status-helper.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { withReadingStatus, pickFresherReadingStatus } from '@/app/library/utils/libraryUtils'; +import type { Book } from '@/types/book'; + +const book: Book = { + hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + createdAt: 1, + updatedAt: 2, + readingStatus: undefined, +}; + +describe('withReadingStatus', () => { + it('sets status, stamps readingStatusUpdatedAt = updatedAt, and does not mutate input', () => { + const out = withReadingStatus(book, 'abandoned'); + expect(out.readingStatus).toBe('abandoned'); + expect(out.readingStatusUpdatedAt).toBe(out.updatedAt); + expect(out.readingStatusUpdatedAt).toBeGreaterThan(0); + expect(book.readingStatus).toBeUndefined(); // input untouched + }); + + it('clears the status when undefined is passed but still stamps the timestamp', () => { + const out = withReadingStatus({ ...book, readingStatus: 'finished' }, undefined); + expect(out.readingStatus).toBeUndefined(); + expect(out.readingStatusUpdatedAt).toBe(out.updatedAt); + }); +}); + +describe('pickFresherReadingStatus', () => { + it('keeps the status whose timestamp is newer, even if the other object is newer overall', () => { + const local = { readingStatus: 'finished' as const, readingStatusUpdatedAt: 200 }; + const remote = { readingStatus: undefined, readingStatusUpdatedAt: 100 }; + expect(pickFresherReadingStatus(local, remote)).toEqual({ + readingStatus: 'finished', + readingStatusUpdatedAt: 200, + }); + }); + + it('treats a missing timestamp as oldest', () => { + const local = { readingStatus: undefined, readingStatusUpdatedAt: undefined }; + const remote = { readingStatus: 'abandoned' as const, readingStatusUpdatedAt: 5 }; + expect(pickFresherReadingStatus(local, remote)).toEqual({ + readingStatus: 'abandoned', + readingStatusUpdatedAt: 5, + }); + }); + + it('prefers the first argument on a timestamp tie', () => { + const a = { readingStatus: 'reading' as const, readingStatusUpdatedAt: 50 }; + const b = { readingStatus: 'finished' as const, readingStatusUpdatedAt: 50 }; + expect(pickFresherReadingStatus(a, b).readingStatus).toBe('reading'); + }); +}); diff --git a/apps/readest-app/src/__tests__/app/library/status-badge.test.tsx b/apps/readest-app/src/__tests__/app/library/status-badge.test.tsx new file mode 100644 index 00000000..694079dc --- /dev/null +++ b/apps/readest-app/src/__tests__/app/library/status-badge.test.tsx @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest'; +import { render } from '@testing-library/react'; +import StatusBadge from '@/app/library/components/StatusBadge'; + +describe('StatusBadge', () => { + it('renders children for the abandoned status', () => { + const { queryByText } = render(On hold); + expect(queryByText('On hold')).not.toBeNull(); + }); + + it('renders nothing for the reading status', () => { + const { container } = render(x); + expect(container.firstChild).toBeNull(); + }); + + it('renders nothing when status is undefined', () => { + const { container } = render(x); + expect(container.firstChild).toBeNull(); + }); +}); diff --git a/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts b/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts new file mode 100644 index 00000000..66f0a02b --- /dev/null +++ b/apps/readest-app/src/__tests__/pages/api/sync-reading-status.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { resolveReadingStatusMerge } from '@/pages/api/sync'; + +const iso = (ms: number) => new Date(ms).toISOString(); + +describe('resolveReadingStatusMerge', () => { + it('keeps the client status when its status timestamp is newer', () => { + const out = resolveReadingStatusMerge( + { reading_status: 'finished', reading_status_updated_at: iso(200) }, + { reading_status: 'reading', reading_status_updated_at: iso(100) }, + ); + expect(out).toEqual({ reading_status: 'finished', reading_status_updated_at: iso(200) }); + }); + + it('keeps the server status when its status timestamp is newer', () => { + const out = resolveReadingStatusMerge( + { reading_status: 'reading', reading_status_updated_at: iso(100) }, + { reading_status: 'finished', reading_status_updated_at: iso(300) }, + ); + expect(out).toEqual({ reading_status: 'finished', reading_status_updated_at: iso(300) }); + }); + + it('treats a missing timestamp as oldest (server wins over an unstamped client)', () => { + const out = resolveReadingStatusMerge( + { reading_status: undefined, reading_status_updated_at: undefined }, + { reading_status: 'abandoned', reading_status_updated_at: iso(1) }, + ); + expect(out).toEqual({ reading_status: 'abandoned', reading_status_updated_at: iso(1) }); + }); +}); diff --git a/apps/readest-app/src/__tests__/store/library-store.test.ts b/apps/readest-app/src/__tests__/store/library-store.test.ts index 1d24e261..a12b38ea 100644 --- a/apps/readest-app/src/__tests__/store/library-store.test.ts +++ b/apps/readest-app/src/__tests__/store/library-store.test.ts @@ -163,6 +163,25 @@ describe('libraryStore', () => { const visible = useLibraryStore.getState().getVisibleLibrary(); expect(visible.map((b) => b.hash)).toEqual(['a', 'c']); }); + + test('stamps readingStatusUpdatedAt when the status changes', () => { + useLibraryStore.getState().setLibrary([makeBook({ hash: 'a', readingStatus: undefined })]); + useLibraryStore.getState().updateBookProgress('a', [100, 100], 'finished'); + const book = useLibraryStore.getState().getBookByHash('a'); + expect(book?.readingStatus).toBe('finished'); + expect(book?.readingStatusUpdatedAt).toBeGreaterThan(0); + }); + + test('does NOT change readingStatusUpdatedAt on a progress-only update', () => { + useLibraryStore + .getState() + .setLibrary([ + makeBook({ hash: 'a', readingStatus: 'reading', readingStatusUpdatedAt: 111 }), + ]); + useLibraryStore.getState().updateBookProgress('a', [50, 100], 'reading'); + const book = useLibraryStore.getState().getBookByHash('a'); + expect(book?.readingStatusUpdatedAt).toBe(111); + }); }); describe('updateBooks', () => { diff --git a/apps/readest-app/src/__tests__/utils/transform.test.ts b/apps/readest-app/src/__tests__/utils/transform.test.ts index f4a12f95..dd2679ad 100644 --- a/apps/readest-app/src/__tests__/utils/transform.test.ts +++ b/apps/readest-app/src/__tests__/utils/transform.test.ts @@ -4,8 +4,10 @@ import { transformBookNoteFromDB, transformBookConfigToDB, transformBookConfigFromDB, + transformBookToDB, + transformBookFromDB, } from '@/utils/transform'; -import { BookConfig, BookNote } from '@/types/book'; +import { BookConfig, BookNote, Book } from '@/types/book'; import { DBBookConfig, DBBookNote } from '@/types/records'; describe('transformBookNoteToDB with xpointer fields', () => { @@ -284,3 +286,56 @@ describe('transformBookConfigToDB / transformBookConfigFromDB rsvpPosition', () expect(restored.rsvpPosition).toEqual(config.rsvpPosition); }); }); + +describe('transformBook readingStatus + readingStatusUpdatedAt', () => { + const userId = 'user-1'; + const baseBook: Book = { + hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + createdAt: 1, + updatedAt: 2, + }; + + it('serializes abandoned status + timestamp to ISO in the DB record', () => { + const ts = Date.UTC(2026, 5, 18, 12, 0, 0); + const db = transformBookToDB( + { ...baseBook, readingStatus: 'abandoned', readingStatusUpdatedAt: ts }, + userId, + ); + expect(db.reading_status).toBe('abandoned'); + expect(db.reading_status_updated_at).toBe(new Date(ts).toISOString()); + }); + + it('leaves reading_status_updated_at null when unset', () => { + const db = transformBookToDB({ ...baseBook, readingStatus: 'finished' }, userId); + expect(db.reading_status_updated_at).toBeNull(); + }); + + it('round-trips abandoned + timestamp back to the client shape', () => { + const ts = Date.UTC(2026, 5, 18, 12, 0, 0); + const db = transformBookToDB( + { ...baseBook, readingStatus: 'abandoned', readingStatusUpdatedAt: ts }, + userId, + ); + const back = transformBookFromDB(db); + expect(back.readingStatus).toBe('abandoned'); + expect(back.readingStatusUpdatedAt).toBe(ts); + }); + + it('reads undefined readingStatusUpdatedAt when the DB column is null', () => { + const back = transformBookFromDB({ + user_id: userId, + book_hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + reading_status: 'finished', + reading_status_updated_at: null, + created_at: new Date(1).toISOString(), + updated_at: new Date(2).toISOString(), + }); + expect(back.readingStatusUpdatedAt).toBeUndefined(); + }); +}); diff --git a/apps/readest-app/src/app/library/components/Bookshelf.tsx b/apps/readest-app/src/app/library/components/Bookshelf.tsx index 359c0864..33396bf6 100644 --- a/apps/readest-app/src/app/library/components/Bookshelf.tsx +++ b/apps/readest-app/src/app/library/components/Bookshelf.tsx @@ -43,6 +43,7 @@ import { compareSortValues, resolveEffectivePrimarySort, resolveEffectiveSecondarySort, + withReadingStatus, } from '../utils/libraryUtils'; import { eventDispatcher } from '@/utils/event'; import { getLocalBookFilename } from '@/utils/book'; @@ -542,7 +543,7 @@ const Bookshelf: React.FC = ({ for (const id of selectedIds) { const book = filteredBooks.find((b) => b.hash === id); if (book) { - booksToUpdate.push({ ...book, readingStatus: status, updatedAt: Date.now() }); + booksToUpdate.push(withReadingStatus(book, status)); } } @@ -557,7 +558,7 @@ const Bookshelf: React.FC = ({ const handleUpdateReadingStatus = useCallback( async (book: Book, status: ReadingStatus | undefined) => { - const updatedBook = { ...book, readingStatus: status, updatedAt: Date.now() }; + const updatedBook = withReadingStatus(book, status); await updateBooks(envConfig, [updatedBook]); }, [envConfig, updateBooks], diff --git a/apps/readest-app/src/app/library/components/BookshelfItem.tsx b/apps/readest-app/src/app/library/components/BookshelfItem.tsx index 54a6fbd8..731640b9 100644 --- a/apps/readest-app/src/app/library/components/BookshelfItem.tsx +++ b/apps/readest-app/src/app/library/components/BookshelfItem.tsx @@ -254,6 +254,12 @@ const BookshelfItem: React.FC = ({ handleUpdateReadingStatus(book, 'unread'); }, }, + markAbandoned: { + text: _('Mark as On hold'), + action: async () => { + handleUpdateReadingStatus(book, 'abandoned'); + }, + }, clearStatus: { text: _('Clear Status'), action: async () => { diff --git a/apps/readest-app/src/app/library/components/ReadingProgress.tsx b/apps/readest-app/src/app/library/components/ReadingProgress.tsx index 285f4180..e407394e 100644 --- a/apps/readest-app/src/app/library/components/ReadingProgress.tsx +++ b/apps/readest-app/src/app/library/components/ReadingProgress.tsx @@ -33,6 +33,20 @@ const ReadingProgress: React.FC = memo( ); } + if (book.readingStatus === 'abandoned') { + return ( +
+ {_('On hold')} + {progressPercentage !== null && !Number.isNaN(progressPercentage) && ( + {progressPercentage}% + )} +
+ ); + } + if (book.readingStatus === 'unread') { if (SHOW_UNREAD_STATUS_BADGE) { return ( diff --git a/apps/readest-app/src/app/library/components/SetStatusAlert.tsx b/apps/readest-app/src/app/library/components/SetStatusAlert.tsx index 6a7b4476..69baaf43 100644 --- a/apps/readest-app/src/app/library/components/SetStatusAlert.tsx +++ b/apps/readest-app/src/app/library/components/SetStatusAlert.tsx @@ -34,6 +34,12 @@ const SetStatusAlert: React.FC = ({ className: 'not-eink:bg-success/15 not-eink:text-success not-eink:border-success/20 eink-bordered', }, + { + label: _('Mark as On hold'), + status: 'abandoned' as ReadingStatus, + className: + 'not-eink:bg-slate-500/15 not-eink:text-slate-600 dark:not-eink:text-slate-300 not-eink:border-slate-500/20 eink-bordered', + }, { label: _('Clear Status'), status: undefined, diff --git a/apps/readest-app/src/app/library/components/StatusBadge.tsx b/apps/readest-app/src/app/library/components/StatusBadge.tsx index acb06ad3..0003e00d 100644 --- a/apps/readest-app/src/app/library/components/StatusBadge.tsx +++ b/apps/readest-app/src/app/library/components/StatusBadge.tsx @@ -8,9 +8,7 @@ interface StatusBadgeProps { } const StatusBadge: React.FC = ({ status, children, className }) => { - if (status !== 'finished' && status !== 'unread') return null; - - const isFinished = status === 'finished'; + if (status !== 'finished' && status !== 'unread' && status !== 'abandoned') return null; return ( = ({ status, children, className } 'rounded-[1px] px-0.5', 'text-[8px] font-bold uppercase leading-none tracking-wider', 'h-3.5', - isFinished && 'status-badge-finished', - !isFinished && 'status-badge-unread', + status === 'finished' && 'status-badge-finished', + status === 'unread' && 'status-badge-unread', + status === 'abandoned' && 'status-badge-abandoned', // finished: green/emerald - isFinished && 'bg-emerald-100 dark:bg-emerald-900/90', - isFinished && 'border border-emerald-300/50 dark:border-emerald-700/50', - isFinished && 'text-emerald-700 dark:text-emerald-300', + status === 'finished' && 'bg-emerald-100 dark:bg-emerald-900/90', + status === 'finished' && 'border border-emerald-300/50 dark:border-emerald-700/50', + status === 'finished' && 'text-emerald-700 dark:text-emerald-300', // unread: pastel yellow/amber - !isFinished && 'bg-amber-100 dark:bg-amber-900/80', - !isFinished && 'border border-amber-300/50 dark:border-amber-700/50', - !isFinished && 'text-amber-700 dark:text-amber-300', + status === 'unread' && 'bg-amber-100 dark:bg-amber-900/80', + status === 'unread' && 'border border-amber-300/50 dark:border-amber-700/50', + status === 'unread' && 'text-amber-700 dark:text-amber-300', + // abandoned / on hold: slate + status === 'abandoned' && 'bg-slate-100 dark:bg-slate-800/80', + status === 'abandoned' && 'border border-slate-300/50 dark:border-slate-600/50', + status === 'abandoned' && 'text-slate-700 dark:text-slate-300', className, )} role='status' diff --git a/apps/readest-app/src/app/library/hooks/useBooksSync.ts b/apps/readest-app/src/app/library/hooks/useBooksSync.ts index 1d4901b8..7fce895d 100644 --- a/apps/readest-app/src/app/library/hooks/useBooksSync.ts +++ b/apps/readest-app/src/app/library/hooks/useBooksSync.ts @@ -9,6 +9,7 @@ import { SYNC_BOOKS_INTERVAL_SEC } from '@/services/constants'; import { throttle } from '@/utils/throttle'; import { debounce } from '@/utils/debounce'; import { eventDispatcher } from '@/utils/event'; +import { pickFresherReadingStatus } from '@/app/library/utils/libraryUtils'; export const useBooksSync = () => { const _ = useTranslation(); @@ -129,6 +130,11 @@ export const useBooksSync = () => { matchingBook.updatedAt >= oldBook.updatedAt ? { ...oldBook, ...matchingBook, syncedAt: Date.now() } : { ...matchingBook, ...oldBook, syncedAt: Date.now() }; + // Status is resolved by its own timestamp, independent of the row's + // updatedAt (which page-turn progress dominates) — see #4634. + const status = pickFresherReadingStatus(oldBook, matchingBook); + mergedBook.readingStatus = status.readingStatus; + mergedBook.readingStatusUpdatedAt = status.readingStatusUpdatedAt; return mergedBook; } return oldBook; diff --git a/apps/readest-app/src/app/library/utils/libraryUtils.ts b/apps/readest-app/src/app/library/utils/libraryUtils.ts index dee05380..3b78d1f2 100644 --- a/apps/readest-app/src/app/library/utils/libraryUtils.ts +++ b/apps/readest-app/src/app/library/utils/libraryUtils.ts @@ -1,4 +1,4 @@ -import { Book, BooksGroup } from '@/types/book'; +import { Book, BooksGroup, ReadingStatus } from '@/types/book'; import { LibraryGroupByType, LibrarySecondarySortByType, @@ -597,6 +597,7 @@ export type BookContextMenuItemId = | 'group' | 'markFinished' | 'markUnread' + | 'markAbandoned' | 'clearStatus' | 'showDetails' | 'showInFinder' @@ -606,6 +607,37 @@ export type BookContextMenuItemId = | 'share' | 'delete'; +/** + * Build a new Book with an explicit reading status. Stamps both `updatedAt` + * (so the library sync picks it up) and `readingStatusUpdatedAt` (so the + * field-level merge resolves status independently of progress). Use this for + * every deliberate status edit so the timestamp is never forgotten. + */ +export const withReadingStatus = (book: Book, status: ReadingStatus | undefined): Book => { + const now = Date.now(); + return { ...book, readingStatus: status, readingStatusUpdatedAt: now, updatedAt: now }; +}; + +type ReadingStatusFields = Pick; + +/** + * Field-level last-writer-wins for reading status: return whichever side's + * status was set more recently (ties → `a`). Missing timestamp = epoch 0. + * The book row's `updatedAt` is dominated by page-turn progress, so status + * must be resolved by its own timestamp or progress would clobber it. + */ +export const pickFresherReadingStatus = ( + a: ReadingStatusFields, + b: ReadingStatusFields, +): ReadingStatusFields => { + const at = (x: ReadingStatusFields) => x.readingStatusUpdatedAt ?? 0; + const winner = at(a) >= at(b) ? a : b; + return { + readingStatus: winner.readingStatus, + readingStatusUpdatedAt: winner.readingStatusUpdatedAt, + }; +}; + /** * Resolve the ordered list of context-menu item ids for a book from its state. * @@ -617,8 +649,13 @@ export type BookContextMenuItemId = export const getBookContextMenuItemIds = (book: Book): BookContextMenuItemId[] => { const ids: BookContextMenuItemId[] = ['select', 'group']; ids.push(book.readingStatus === 'finished' ? 'markUnread' : 'markFinished'); + if (book.readingStatus !== 'abandoned') ids.push('markAbandoned'); // "Clear Status" is offered only when the book has an explicit status set. - if (book.readingStatus === 'finished' || book.readingStatus === 'unread') { + if ( + book.readingStatus === 'finished' || + book.readingStatus === 'unread' || + book.readingStatus === 'abandoned' + ) { ids.push('clearStatus'); } ids.push('showDetails', 'showInFinder', 'searchGoodreads'); diff --git a/apps/readest-app/src/pages/api/sync.ts b/apps/readest-app/src/pages/api/sync.ts index 73ce85b3..46e21c77 100644 --- a/apps/readest-app/src/pages/api/sync.ts +++ b/apps/readest-app/src/pages/api/sync.ts @@ -37,6 +37,29 @@ export function pickWinningPages( return { toUpsert }; } +/** + * Field-level last-writer-wins for a books row's reading_status: return the + * status fields with the newer reading_status_updated_at (ties → client). NULL + * timestamp = epoch 0. Lets reading_status survive even when the whole row is + * decided the other way by updated_at (which page-turn progress dominates) — + * issue #4634. + */ +export function resolveReadingStatusMerge( + client: Pick, + server: Pick, +): Pick { + const ms = (s?: string | null) => (s ? new Date(s).getTime() : 0); + return ms(client.reading_status_updated_at) >= ms(server.reading_status_updated_at) + ? { + reading_status: client.reading_status, + reading_status_updated_at: client.reading_status_updated_at, + } + : { + reading_status: server.reading_status, + reading_status_updated_at: server.reading_status_updated_at, + }; +} + const transformsToDB = { books: transformBookToDB, book_notes: transformBookNoteToDB, @@ -378,7 +401,43 @@ export async function POST(req: NextRequest) { const clientIsNewer = clientDeletedAt > serverDeletedAt || clientUpdatedAt > serverUpdatedAt; - if (clientIsNewer) { + if (table === 'books') { + // `dbRec` is DBBook | DBBookConfig; in the 'books' branch it is always DBBook. + const clientBook = dbRec as DBBook; + // `serverData` is BookDataRecord but the DB row carries the status columns at + // runtime — widen the type without going through `unknown`. + const serverBook = serverData as BookDataRecord & + Partial>; + const status = resolveReadingStatusMerge(clientBook, serverBook); + if (clientIsNewer) { + // Client wins the row; graft the fresher status onto it (server's + // status may be the newer one even though the row is older). + clientBook.reading_status = status.reading_status; + clientBook.reading_status_updated_at = status.reading_status_updated_at; + toUpdate.push(clientBook); + } else { + // Only rewrite when the resolved status VALUE differs from the + // server's — a timestamp-only difference on the same value is a + // no-op, and rewriting it would churn updated_at + re-propagate. + const statusChanged = status.reading_status !== serverBook.reading_status; + if (statusChanged) { + // Server wins the row, but the client's status is newer. Write + // server's row with the fresher status and bump updated_at so + // peers re-pull the status change. + // The runtime DB row carries all DBBook columns; the static type + // of `serverBook` is a narrower intersection so `unknown` is + // required to bridge the gap at this one construction site. + toUpdate.push({ + ...serverBook, + reading_status: status.reading_status, + reading_status_updated_at: status.reading_status_updated_at, + updated_at: new Date().toISOString(), + } as unknown as DBBook); + } else { + batchAuthoritativeRecords.push(serverData); + } + } + } else if (clientIsNewer) { toUpdate.push(dbRec); } else { batchAuthoritativeRecords.push(serverData); diff --git a/apps/readest-app/src/store/libraryStore.ts b/apps/readest-app/src/store/libraryStore.ts index 90e2b264..a0214b20 100644 --- a/apps/readest-app/src/store/libraryStore.ts +++ b/apps/readest-app/src/store/libraryStore.ts @@ -107,10 +107,12 @@ export const useLibraryStore = create((set, get) => ({ const idx = hashIndex.get(hash); if (idx === undefined) return; const book = library[idx]!; + const statusChanged = readingStatus !== book.readingStatus; const updatedBook: Book = { ...book, progress, readingStatus, + readingStatusUpdatedAt: statusChanged ? Date.now() : book.readingStatusUpdatedAt, updatedAt: Date.now(), }; const newLibrary = library.slice(); diff --git a/apps/readest-app/src/types/book.ts b/apps/readest-app/src/types/book.ts index c3ba5689..2a23a2a8 100644 --- a/apps/readest-app/src/types/book.ts +++ b/apps/readest-app/src/types/book.ts @@ -16,7 +16,7 @@ export type BookFormat = | 'TXT' | 'MD'; export type BookNoteType = 'bookmark' | 'annotation' | 'excerpt'; -export type ReadingStatus = 'unread' | 'reading' | 'finished'; +export type ReadingStatus = 'unread' | 'reading' | 'finished' | 'abandoned'; export type HighlightStyle = 'highlight' | 'underline' | 'squiggly'; // Predefined highlight colors, can be extended with custom hex colors export type HighlightColor = 'red' | 'yellow' | 'green' | 'blue' | 'violet' | string; @@ -112,6 +112,7 @@ export interface Book { lastUpdated?: number; // deprecated in favor of updatedAt progress?: [number, number]; // Add progress field: [current, total], 1-based page number readingStatus?: ReadingStatus; + readingStatusUpdatedAt?: number; // ms; bumped only when readingStatus changes primaryLanguage?: string; metadata?: BookMetadata; diff --git a/apps/readest-app/src/types/records.ts b/apps/readest-app/src/types/records.ts index 983ff8e8..96ed1dc0 100644 --- a/apps/readest-app/src/types/records.ts +++ b/apps/readest-app/src/types/records.ts @@ -11,6 +11,7 @@ export interface DBBook { tags?: string[]; progress?: [number, number]; reading_status?: string; + reading_status_updated_at?: string | null; metadata?: string | null; created_at?: string; diff --git a/apps/readest-app/src/utils/transform.ts b/apps/readest-app/src/utils/transform.ts index 2eaf92b0..3436fb7b 100644 --- a/apps/readest-app/src/utils/transform.ts +++ b/apps/readest-app/src/utils/transform.ts @@ -76,6 +76,7 @@ export const transformBookToDB = (book: unknown, userId: string): DBBook => { tags, progress, readingStatus, + readingStatusUpdatedAt, metadata, createdAt, updatedAt, @@ -95,6 +96,9 @@ export const transformBookToDB = (book: unknown, userId: string): DBBook => { tags: tags, progress: progress, reading_status: readingStatus, + reading_status_updated_at: readingStatusUpdatedAt + ? new Date(readingStatusUpdatedAt).toISOString() + : null, source_title: sanitizeString(sourceTitle), metadata: metadata ? sanitizeString(JSON.stringify(metadata)) : null, created_at: new Date(createdAt ?? Date.now()).toISOString(), @@ -116,6 +120,7 @@ export const transformBookFromDB = (dbBook: DBBook): Book => { tags, progress, reading_status, + reading_status_updated_at, source_title, metadata, created_at, @@ -135,6 +140,9 @@ export const transformBookFromDB = (dbBook: DBBook): Book => { tags: tags, progress: progress, readingStatus: reading_status as ReadingStatus, + readingStatusUpdatedAt: reading_status_updated_at + ? new Date(reading_status_updated_at).getTime() + : undefined, sourceTitle: source_title, metadata: metadata ? JSON.parse(metadata) : null, createdAt: new Date(created_at!).getTime(), diff --git a/apps/readest.koplugin/library/librarystore.lua b/apps/readest.koplugin/library/librarystore.lua index 219714ac..8fc68b63 100644 --- a/apps/readest.koplugin/library/librarystore.lua +++ b/apps/readest.koplugin/library/librarystore.lua @@ -13,7 +13,7 @@ local SQ3 = require("lua-ljsqlite3/init") local json = require("json") -local SCHEMA_VERSION = 1 +local SCHEMA_VERSION = 2 local SCHEMA_SQL = [[ CREATE TABLE IF NOT EXISTS books ( @@ -36,6 +36,7 @@ CREATE TABLE IF NOT EXISTS books ( uploaded_at INTEGER, progress_lib TEXT, reading_status TEXT, + reading_status_updated_at INTEGER, last_read_at INTEGER, created_at INTEGER, updated_at INTEGER, @@ -62,7 +63,7 @@ local BOOK_COLS = { "format", "metadata_json", "series", "series_index", "group_id", "group_name", "cover_path", "file_path", "cloud_present", "local_present", "uploaded_at", "progress_lib", "reading_status", - "last_read_at", "created_at", "updated_at", "deleted_at", + "reading_status_updated_at", "last_read_at", "created_at", "updated_at", "deleted_at", } local BOOK_COL_INDEX = {} for i, c in ipairs(BOOK_COLS) do BOOK_COL_INDEX[c] = i end @@ -73,8 +74,8 @@ for i, c in ipairs(BOOK_COLS) do BOOK_COL_INDEX[c] = i end -- well within Lua's 53-bit double mantissa. local NUMERIC_COLS = { series_index = true, cloud_present = true, local_present = true, - uploaded_at = true, last_read_at = true, created_at = true, - updated_at = true, deleted_at = true, + uploaded_at = true, reading_status_updated_at = true, last_read_at = true, + created_at = true, updated_at = true, deleted_at = true, } local function row_to_table(raw) @@ -136,8 +137,24 @@ function M.new(opts) self.user_id = opts.user_id self.db_path = opts.db_path or ":memory:" self.db = SQ3.open(self.db_path) + -- Read version before creating tables; getUserVersion uses rowexec which + -- may leave an open iterator in some SQLite bindings, so use prepare/step. + local prev_stmt = self.db:prepare("PRAGMA user_version;") + local prev_row = prev_stmt:reset():step() + prev_stmt:close() + local prev = prev_row and tonumber(prev_row[1]) or 0 self.db:exec(SCHEMA_SQL) - self.db:exec(string.format("PRAGMA user_version = %d;", SCHEMA_VERSION)) + -- v1 -> v2: add reading_status_updated_at to existing DBs. CREATE TABLE + -- IF NOT EXISTS won't add a column, so ALTER it in (pcall guards a DB that + -- somehow already has the column). + if prev >= 1 and prev < 2 then + pcall(function() + self.db:exec("ALTER TABLE books ADD COLUMN reading_status_updated_at INTEGER;") + end) + end + if prev < SCHEMA_VERSION then + self.db:exec(string.format("PRAGMA user_version = %d;", SCHEMA_VERSION)) + end self._groups_cache = {} return self end @@ -745,6 +762,10 @@ function M.parseSyncRow(dbRow) -- Reading status passthrough (web side has 'unread'/'reading'/'finished') out.reading_status = dbRow.readingStatus or dbRow.reading_status + -- ms; server sends it as a timestamptz ISO string (iso_to_ms also passes + -- through a raw number when a caller already supplied ms). + out.reading_status_updated_at = iso_to_ms(dbRow.reading_status_updated_at) + or iso_to_ms(dbRow.readingStatusUpdatedAt) -- Cloud-presence flag: tombstones from the cloud arrive with deleted_at -- set; the row is still useful for tracking that the cloud copy is gone, diff --git a/apps/readest.koplugin/library/librarywidget.lua b/apps/readest.koplugin/library/librarywidget.lua index 73cac56c..5d147837 100644 --- a/apps/readest.koplugin/library/librarywidget.lua +++ b/apps/readest.koplugin/library/librarywidget.lua @@ -471,22 +471,54 @@ end -- --------------------------------------------------------------------------- local function runCloudSync(opts, store) local mode = opts.settings.auto_sync and "both" or "pull" + local DocSettings = require("docsettings") + local BookList = require("ui/widget/booklist") + local statussync = require("library.statussync") + local deps = { + now_ms = function() return os.time() * 1000 end, + open_summary = function(file_path) + local ok, ds = pcall(DocSettings.open, DocSettings, file_path) + if not ok or not ds then return nil end + return ds:readSetting("summary") + end, + write_status = function(file_path, ko_status) + local ok, ds = pcall(DocSettings.open, DocSettings, file_path) + if not ok or not ds then return end + local summary = ds:readSetting("summary") or {} + summary.status = ko_status -- nil clears -> KOReader "New" + summary.modified = os.date("%Y-%m-%d", os.time()) + ds:saveSetting("summary", summary) + ds:flush() + BookList.setBookInfoCacheProperty(file_path, "status", ko_status) + end, + } + local function reconcile() statussync.reconcileLocalStatuses(store, deps) end + logger.info("ReadestLibrary runCloudSync: mode=" .. mode .. " auto_sync=" .. tostring(opts.settings.auto_sync)) - syncbooks.syncBooks({ - sync_auth = opts.sync_auth, - sync_path = opts.sync_path, - settings = opts.settings, - store = store, - }, mode, function(success, msg, status) + + local function done(success, msg, status) logger.info("ReadestLibrary runCloudSync[" .. mode .. "] done: success=" - .. tostring(success) .. " msg=" .. tostring(msg) - .. " status=" .. tostring(status)) - -- Refresh either way: success picks up new cloud rows; failure - -- (auth, network, server) leaves local rows visible without - -- leaking a stale display state. + .. tostring(success) .. " msg=" .. tostring(msg) .. " status=" .. tostring(status)) M.refresh() - end) + end + + if mode == "both" then + -- before_push runs after pull, before push: apply pulled statuses to + -- sidecars and capture sidecar changes into the store so they're pushed. + syncbooks.syncBooks({ + sync_auth = opts.sync_auth, sync_path = opts.sync_path, + settings = opts.settings, store = store, + }, "both", done, reconcile) + else + syncbooks.syncBooks({ + sync_auth = opts.sync_auth, sync_path = opts.sync_path, + settings = opts.settings, store = store, + }, "pull", function(success, msg, status) + reconcile() -- apply cloud statuses to sidecars even when auto_sync is off + done(success, msg, status) + end) + end end -- Cloud sync HTTP is synchronous on platforms without the Turbo looper diff --git a/apps/readest.koplugin/library/readingstatus.lua b/apps/readest.koplugin/library/readingstatus.lua new file mode 100644 index 00000000..9d3bdc8a --- /dev/null +++ b/apps/readest.koplugin/library/readingstatus.lua @@ -0,0 +1,108 @@ +-- readingstatus.lua — pure bidirectional mapping + reconcile between Readest's +-- reading_status and KOReader's summary.status. No KOReader globals, so it +-- unit-tests cleanly under busted. +-- +-- ONLY DELIBERATE statuses sync. KOReader auto-sets summary.status = "reading" +-- the first time a book is opened, so "reading" (and "New"/absent) is treated as +-- NON-DECISIVE and never captured — otherwise opening a finished book on +-- KOReader would downgrade it. Reading *position* syncs via the progress +-- channel, not here. +-- +-- Decisive: Readest unread / finished / abandoned +-- KOReader complete / abandoned +-- +-- On the unsynced baseline (a book whose Readest reading_status_updated_at is +-- 0/absent — status predates this feature or was pulled before it) timestamps +-- aren't trustworthy, so conflicts resolve "Readest authoritative". The first +-- reconcile of such a book stamps reading_status_updated_at = now_ms, which +-- exits bootstrap; every later change then resolves by ordinary recency LWW. +local M = {} + +-- Readest reading_status -> KOReader summary.status push target. +-- finished->complete, abandoned->abandoned, unread->nil (clear / "New"). +-- 'reading'/undefined are non-decisive and never pushed. +local READEST_TO_KO = { finished = "complete", abandoned = "abandoned" } +function M.readest_to_ko(status) + if status == "unread" then return nil end + return READEST_TO_KO[status] +end + +-- KOReader summary.status -> Readest reading_status, ONLY for decisive KO +-- statuses. "reading" (auto-on-open), "New", nil, unknown -> nil (no opinion). +local KO_TO_READEST = { complete = "finished", abandoned = "abandoned" } +function M.ko_to_readest(status) + return KO_TO_READEST[status] +end + +-- Is a Readest status a deliberate signal worth syncing? +function M.readest_decisive(status) + return status == "unread" or status == "finished" or status == "abandoned" +end + +-- "YYYY-MM-DD" -> unix ms at local midnight; nil if unparseable. +function M.parse_modified_ms(s) + if type(s) ~= "string" then return nil end + local y, mo, d = s:match("^(%d%d%d%d)%-(%d%d)%-(%d%d)") + if not y then return nil end + local t = os.time({ year = tonumber(y), month = tonumber(mo), day = tonumber(d), + hour = 0, min = 0, sec = 0 }) + if not t then return nil end + return t * 1000 +end + +-- Decide what to write so both sides converge on the winning decisive status. +-- cloud = { reading_status, reading_status_updated_at(ms) } (LibraryStore row) +-- ko = { status (KO summary.status), ts (ms, from summary.modified) } +-- now_ms = current time in ms (used for the bootstrap stamp) +-- Returns { write_ko, write_store, readest_status, ts, ko_status } where +-- write_ko => set the sidecar to ko_status (may be nil = clear), +-- write_store => set the LibraryStore row to (readest_status, ts). +-- Both false means "nothing to do". +function M.reconcile(cloud, ko, now_ms) + cloud = cloud or {} + ko = ko or {} + now_ms = now_ms or 0 + + local cr = cloud.reading_status + local cloud_ts = cloud.reading_status_updated_at or 0 + local cr_dec = M.readest_decisive(cr) + + local kr = M.ko_to_readest(ko.status) -- finished | abandoned | nil + local ko_ts = ko.ts or 0 + local ko_dec = kr ~= nil + + -- Neither side has a decisive status: nothing to sync or baseline. + if not cr_dec and not ko_dec then + return { write_ko = false, write_store = false } + end + + -- Winning Readest status W and its authoritative timestamp W_ts. + local W, W_ts + if cr_dec and not ko_dec then + W, W_ts = cr, (cloud_ts > 0 and cloud_ts or now_ms) + elseif ko_dec and not cr_dec then + W, W_ts = kr, (ko_ts > 0 and ko_ts or now_ms) + elseif cr == kr then -- both decisive, already agree + W, W_ts = cr, (cloud_ts > 0 and cloud_ts or now_ms) + elseif cloud_ts == 0 then -- bootstrap conflict: Readest authoritative + W, W_ts = cr, now_ms + elseif cloud_ts >= ko_ts then -- steady-state LWW (tie -> Readest) + W, W_ts = cr, cloud_ts + else + W, W_ts = kr, ko_ts + end + + -- Equalize both sides to W. + local target_ko = M.readest_to_ko(W) -- complete | abandoned | nil (clear) + local write_ko = ko.status ~= target_ko + local write_store = (cr ~= W) or (cloud_ts ~= W_ts) + return { + write_ko = write_ko, + write_store = write_store, + readest_status = W, + ts = W_ts, + ko_status = target_ko, + } +end + +return M diff --git a/apps/readest.koplugin/library/statussync.lua b/apps/readest.koplugin/library/statussync.lua new file mode 100644 index 00000000..a3d17bd4 --- /dev/null +++ b/apps/readest.koplugin/library/statussync.lua @@ -0,0 +1,45 @@ +-- statussync.lua — bridge LibraryStore.reading_status <-> KOReader's per-book +-- summary.status. The decision is delegated to the pure readingstatus.reconcile; +-- this module only walks local-present rows and performs the chosen IO. The IO +-- is injected via `deps` so it unit-tests without DocSettings; production wires +-- a DocSettings-backed deps in librarywidget. +local readingstatus = require("library.readingstatus") + +local M = {} + +-- deps: { now_ms(), open_summary(file_path) -> {status, modified}|nil, +-- write_status(file_path, ko_status_or_nil) } +function M.reconcileLocalStatuses(store, deps) + if not store or not deps then return 0 end + -- Capture "now" once so every book baselines at the same instant. + local now_ms = deps.now_ms() + local changed = 0 + local rows = store:listBooks({}) + for _, row in ipairs(rows) do + if row.local_present == 1 and row.file_path then + local summary = deps.open_summary(row.file_path) or {} + local ko_ts = readingstatus.parse_modified_ms(summary.modified) or now_ms + local r = readingstatus.reconcile( + { reading_status = row.reading_status, + reading_status_updated_at = row.reading_status_updated_at }, + { status = summary.status, ts = ko_ts }, + now_ms) + -- Sidecar first (durable), then the store (durable + bumps + -- updated_at for the push). A crash between re-reconciles this book + -- identically next pass — convergent and idempotent. + if r.write_ko then + deps.write_status(row.file_path, r.ko_status) -- ko_status may be nil (clear) + end + if r.write_store then + store:touchBook(row.hash, { + reading_status = r.readest_status, + reading_status_updated_at = r.ts, + }) + end + if r.write_ko or r.write_store then changed = changed + 1 end + end + end + return changed +end + +return M diff --git a/apps/readest.koplugin/library/syncbooks.lua b/apps/readest.koplugin/library/syncbooks.lua index 3c01d2d4..804448ac 100644 --- a/apps/readest.koplugin/library/syncbooks.lua +++ b/apps/readest.koplugin/library/syncbooks.lua @@ -148,6 +148,7 @@ local function row_to_wire(row) groupId = row.group_id, groupName = row.group_name, readingStatus = row.reading_status, + readingStatusUpdatedAt = num(row.reading_status_updated_at), createdAt = num(row.created_at), updatedAt = num(row.updated_at), deletedAt = num(row.deleted_at), diff --git a/apps/readest.koplugin/spec/library/librarystore_spec.lua b/apps/readest.koplugin/spec/library/librarystore_spec.lua index ef0adb2c..ea3adc3f 100644 --- a/apps/readest.koplugin/spec/library/librarystore_spec.lua +++ b/apps/readest.koplugin/spec/library/librarystore_spec.lua @@ -89,11 +89,125 @@ describe("LibraryStore", function() s3:close() end) - it("sets PRAGMA user_version = 1", function() + it("sets PRAGMA user_version = 2", function() local store = LibraryStore.new({ user_id = "alice" }) - assert.are.equal(1, store:getUserVersion()) + assert.are.equal(2, store:getUserVersion()) store:close() end) + + -- ----------------------------------------------------------------- + -- v1 → v2 migration: ALTER TABLE adds reading_status_updated_at + -- ----------------------------------------------------------------- + describe("v1 -> v2 migration", function() + local db_path + local SQ3 = require("lua-ljsqlite3/init") + + after_each(function() + if db_path then + os.remove(db_path) + db_path = nil + end + end) + + it("migrates an existing v1 DB: column added, row preserved, user_version=2", function() + -- Build a temp file path; os.tmpname() creates the file, remove it + -- so sqlite.open() starts fresh instead of opening an existing file. + db_path = os.tmpname() + os.remove(db_path) + + -- Create a v1 DB: books table WITHOUT reading_status_updated_at + -- (all other v2 columns present so SCHEMA_SQL indexes succeed), + -- user_version = 1, one existing row. + local v1db = SQ3.open(db_path) + v1db:exec([[ + CREATE TABLE books ( + user_id TEXT NOT NULL, + hash TEXT NOT NULL, + meta_hash TEXT, + title TEXT NOT NULL, + source_title TEXT, + author TEXT, + format TEXT, + metadata_json TEXT, + series TEXT, + series_index REAL, + group_id TEXT, + group_name TEXT, + cover_path TEXT, + file_path TEXT, + cloud_present INTEGER NOT NULL DEFAULT 0, + local_present INTEGER NOT NULL DEFAULT 0, + uploaded_at INTEGER, + progress_lib TEXT, + reading_status TEXT, + last_read_at INTEGER, + created_at INTEGER, + updated_at INTEGER, + deleted_at INTEGER, + PRIMARY KEY (user_id, hash) + ); + CREATE INDEX IF NOT EXISTS books_user_updated ON books(user_id, updated_at DESC); + CREATE INDEX IF NOT EXISTS books_user_lastread ON books(user_id, last_read_at DESC); + CREATE INDEX IF NOT EXISTS books_user_meta ON books(user_id, meta_hash); + CREATE INDEX IF NOT EXISTS books_user_group ON books(user_id, group_name); + CREATE INDEX IF NOT EXISTS books_user_author ON books(user_id, author); + ]]) + v1db:exec("PRAGMA user_version = 1;") + local ins = v1db:prepare( + "INSERT INTO books (user_id, hash, title, cloud_present, local_present, reading_status) VALUES (?, ?, ?, ?, ?, ?)") + ins:reset():bind("alice", "h-v1", "OldBook", 1, 1, "reading"):step() + ins:close() + v1db:close() + + -- Open the v1 file via LibraryStore; migration should run silently. + local store = LibraryStore.new({ user_id = "alice", db_path = db_path }) + + -- 1. user_version must now be 2 + assert.are.equal(2, store:getUserVersion()) + + -- 2. The pre-existing row is still readable (no error, title intact) + local row = store:_getRowRaw("h-v1") + assert.is_not_nil(row) + assert.are.equal("OldBook", row.title) + assert.are.equal("reading", row.reading_status) + + -- 3. reading_status_updated_at round-trips (column exists and is writable) + store:upsertBook({ + hash = "h-v1", + title = "OldBook", + reading_status_updated_at = 1750000000000, + }) + local row2 = store:_getRowRaw("h-v1") + assert.are.equal(1750000000000, row2.reading_status_updated_at) + + store:close() + end) + end) + end) + + -- ===================================================================== + -- reading_status_updated_at: new column added in schema v2 + -- ===================================================================== + describe("reading_status_updated_at", function() + it("round-trips reading_status_updated_at through upsert + read", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h1", title = "T", reading_status = "finished", + reading_status_updated_at = 1750000000000, local_present = 1 }) + local row = store:_getRowRaw("h1") + assert.are.equal("finished", row.reading_status) + assert.are.equal(1750000000000, row.reading_status_updated_at) + store:close() + end) + + it("parseSyncRow reads reading_status_updated_at from the server ISO field", function() + local parsed = LibraryStore.parseSyncRow({ + book_hash = "h2", title = "T", reading_status = "abandoned", + reading_status_updated_at = "2026-06-18T00:00:00+00:00", updated_at = "2026-06-18T00:00:00+00:00", + }) + -- 2026-06-18T00:00:00Z = 1781740800 unix seconds = 1781740800000 ms + assert.are.equal(1781740800000, parsed.reading_status_updated_at) + assert.are.equal("abandoned", parsed.reading_status) + end) end) -- ===================================================================== diff --git a/apps/readest.koplugin/spec/library/readingstatus_spec.lua b/apps/readest.koplugin/spec/library/readingstatus_spec.lua new file mode 100644 index 00000000..91788c34 --- /dev/null +++ b/apps/readest.koplugin/spec/library/readingstatus_spec.lua @@ -0,0 +1,203 @@ +-- readingstatus_spec.lua — contract for library/readingstatus.lua +require("spec_helper") +local RS = require("library.readingstatus") + +-- A "now" clearly larger than any KOReader summary.modified ms used below, +-- so bootstrap stamps are unambiguous in assertions. +local NOW = 9000000000000 + +describe("readingstatus mapping (decisive-only)", function() + it("maps Readest -> KOReader (push targets)", function() + assert.are.equal("complete", RS.readest_to_ko("finished")) + assert.are.equal("abandoned", RS.readest_to_ko("abandoned")) + assert.is_nil(RS.readest_to_ko("unread")) -- clear -> "New" + assert.is_nil(RS.readest_to_ko("reading")) -- non-decisive, never pushed + assert.is_nil(RS.readest_to_ko(nil)) -- undefined + end) + + it("maps KOReader -> Readest only for decisive statuses", function() + assert.are.equal("finished", RS.ko_to_readest("complete")) + assert.are.equal("abandoned", RS.ko_to_readest("abandoned")) + assert.is_nil(RS.ko_to_readest("reading")) -- auto-set on open: NOT captured + assert.is_nil(RS.ko_to_readest("New")) + assert.is_nil(RS.ko_to_readest(nil)) + end) + + it("knows which Readest statuses are decisive", function() + assert.is_true(RS.readest_decisive("finished")) + assert.is_true(RS.readest_decisive("abandoned")) + assert.is_true(RS.readest_decisive("unread")) + assert.is_false(RS.readest_decisive("reading")) + assert.is_false(RS.readest_decisive(nil)) + end) + + it("parses summary.modified to day ms", function() + assert.are.equal(os.time({ year = 2026, month = 6, day = 18, hour = 0, min = 0, sec = 0 }) * 1000, + RS.parse_modified_ms("2026-06-18")) + assert.is_nil(RS.parse_modified_ms(nil)) + assert.is_nil(RS.parse_modified_ms("garbage")) + end) +end) + +describe("readingstatus reconcile — non-decisive", function() + it("does nothing when neither side has a decisive status", function() + local r = RS.reconcile({ reading_status = nil, reading_status_updated_at = 0 }, + { status = nil, ts = 0 }, NOW) + assert.is_false(r.write_ko) + assert.is_false(r.write_store) + end) + + it("ignores KOReader auto-'reading' against an undefined Readest book (reported case 2)", function() + local r = RS.reconcile({ reading_status = nil, reading_status_updated_at = 0 }, + { status = "reading", ts = 1000 }, NOW) + assert.is_false(r.write_ko) + assert.is_false(r.write_store) + end) +end) + +describe("readingstatus reconcile — only one side decisive", function() + it("pushes a bootstrap Readest 'finished' down to an opened KO book + stamps (reported case 1)", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 0 }, + { status = "reading", ts = 1000 }, NOW) + assert.is_true(r.write_ko) + assert.are.equal("complete", r.ko_status) + assert.is_true(r.write_store) + assert.are.equal("finished", r.readest_status) + assert.are.equal(NOW, r.ts) -- bootstrap stamp + end) + + it("pushes a steady Readest 'finished' down without re-stamping the store", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 500 }, + { status = "reading", ts = 1000 }, NOW) + assert.is_true(r.write_ko) + assert.are.equal("complete", r.ko_status) + assert.is_false(r.write_store) -- already finished@500 + assert.are.equal(500, r.ts) + end) + + it("captures a KO 'complete' into an undefined Readest book at the KO timestamp", function() + local r = RS.reconcile({ reading_status = nil, reading_status_updated_at = 0 }, + { status = "complete", ts = 300 }, NOW) + assert.is_false(r.write_ko) + assert.is_true(r.write_store) + assert.are.equal("finished", r.readest_status) + assert.are.equal(300, r.ts) + end) + + it("captures a KO 'abandoned' even when Readest is in non-decisive 'reading'", function() + local r = RS.reconcile({ reading_status = "reading", reading_status_updated_at = 100 }, + { status = "abandoned", ts = 300 }, NOW) + assert.is_true(r.write_store) + assert.are.equal("abandoned", r.readest_status) + assert.are.equal(300, r.ts) + end) +end) + +describe("readingstatus reconcile — both decisive", function() + it("is a no-op in steady state when both already agree", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 500 }, + { status = "complete", ts = 300 }, NOW) + assert.is_false(r.write_ko) + assert.is_false(r.write_store) + end) + + it("stamps a bootstrap book even when both already agree (exit bootstrap)", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 0 }, + { status = "complete", ts = 300 }, NOW) + assert.is_false(r.write_ko) + assert.is_true(r.write_store) + assert.are.equal("finished", r.readest_status) + assert.are.equal(NOW, r.ts) + end) + + it("bootstrap conflict: Readest is authoritative (finished beats KO on-hold)", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 0 }, + { status = "abandoned", ts = 300 }, NOW) + assert.is_true(r.write_ko) + assert.are.equal("complete", r.ko_status) -- KO pulled to finished + assert.is_true(r.write_store) + assert.are.equal("finished", r.readest_status) + assert.are.equal(NOW, r.ts) + end) + + it("steady conflict: Readest wins when its status timestamp is newer", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 500 }, + { status = "abandoned", ts = 300 }, NOW) + assert.is_true(r.write_ko) + assert.are.equal("complete", r.ko_status) + assert.is_false(r.write_store) + assert.are.equal("finished", r.readest_status) + end) + + it("steady conflict: KOReader wins when the sidecar change is newer", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 300 }, + { status = "abandoned", ts = 500 }, NOW) + assert.is_false(r.write_ko) -- KO already abandoned + assert.is_true(r.write_store) + assert.are.equal("abandoned", r.readest_status) + assert.are.equal(500, r.ts) + end) + + it("bootstrap: a Readest 'unread' reset clears a KO 'complete' (Readest authoritative)", function() + local r = RS.reconcile({ reading_status = "unread", reading_status_updated_at = 0 }, + { status = "complete", ts = 300 }, NOW) + assert.is_true(r.write_ko) + assert.is_nil(r.ko_status) -- clear -> "New" + assert.is_true(r.write_store) + assert.are.equal("unread", r.readest_status) + assert.are.equal(NOW, r.ts) + end) +end) + +describe("readingstatus reconcile — remaining transfer-graph cells", function() + it("unread × reading: pushes a clear to KO + stamps", function() + local r = RS.reconcile({ reading_status = "unread", reading_status_updated_at = 0 }, + { status = "reading", ts = 1000 }, NOW) + assert.is_true(r.write_ko) + assert.is_nil(r.ko_status) -- clear -> "New" + assert.is_true(r.write_store) + assert.are.equal("unread", r.readest_status) + assert.are.equal(NOW, r.ts) + end) + + it("abandoned × reading: pushes 'abandoned' down to KO + stamps", function() + local r = RS.reconcile({ reading_status = "abandoned", reading_status_updated_at = 0 }, + { status = "reading", ts = 1000 }, NOW) + assert.is_true(r.write_ko) + assert.are.equal("abandoned", r.ko_status) + assert.is_true(r.write_store) + assert.are.equal("abandoned", r.readest_status) + end) + + it("abandoned × complete (bootstrap conflict): Readest 'abandoned' wins, KO pulled to abandoned", function() + local r = RS.reconcile({ reading_status = "abandoned", reading_status_updated_at = 0 }, + { status = "complete", ts = 300 }, NOW) + assert.is_true(r.write_ko) + assert.are.equal("abandoned", r.ko_status) + assert.is_true(r.write_store) + assert.are.equal("abandoned", r.readest_status) + assert.are.equal(NOW, r.ts) + end) +end) + +describe("readingstatus reconcile — convergence (no ping-pong)", function() + it("converges after a capture", function() + local r = RS.reconcile({ reading_status = nil, reading_status_updated_at = 0 }, + { status = "complete", ts = 300 }, NOW) + -- equalize: store now holds the captured status; KO already had it + local r2 = RS.reconcile({ reading_status = r.readest_status, reading_status_updated_at = r.ts }, + { status = "complete", ts = 300 }, NOW) + assert.is_false(r2.write_ko) + assert.is_false(r2.write_store) + end) + + it("converges after a bootstrap push", function() + local r = RS.reconcile({ reading_status = "finished", reading_status_updated_at = 0 }, + { status = "reading", ts = 1000 }, NOW) + -- equalize: store stamped to NOW; KO sidecar written to r.ko_status + local r2 = RS.reconcile({ reading_status = r.readest_status, reading_status_updated_at = r.ts }, + { status = r.ko_status, ts = 1000 }, NOW) + assert.is_false(r2.write_ko) + assert.is_false(r2.write_store) + end) +end) diff --git a/apps/readest.koplugin/spec/library/statussync_spec.lua b/apps/readest.koplugin/spec/library/statussync_spec.lua new file mode 100644 index 00000000..15fcee3f --- /dev/null +++ b/apps/readest.koplugin/spec/library/statussync_spec.lua @@ -0,0 +1,81 @@ +require("spec_helper") +local StatusSync = require("library.statussync") +local LibraryStore = require("library.librarystore") + +local function fake_deps(summaries, writes) + return { + now_ms = function() return 1750000000000 end, + open_summary = function(path) return summaries[path] end, -- {status, modified} + write_status = function(path, ko_status) writes[path] = ko_status end, + } +end + +describe("statussync.reconcileLocalStatuses", function() + it("applies a newer cloud status down to the sidecar", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h1", title = "T", file_path = "/b1.epub", local_present = 1, + reading_status = "finished", reading_status_updated_at = 1770000000000 }) + local summaries = { ["/b1.epub"] = { status = "reading", modified = "2026-01-01" } } + local writes = {} + StatusSync.reconcileLocalStatuses(store, fake_deps(summaries, writes)) + assert.are.equal("complete", writes["/b1.epub"]) + store:close() + end) + + it("captures a newer sidecar status into the store and bumps updated_at", function() + local RS = require("library.readingstatus") + local expected_ts = RS.parse_modified_ms("2026-06-18") + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h2", title = "T", file_path = "/b2.epub", local_present = 1, + reading_status = "reading", reading_status_updated_at = 100 }) + local summaries = { ["/b2.epub"] = { status = "complete", modified = "2026-06-18" } } + StatusSync.reconcileLocalStatuses(store, fake_deps(summaries, {})) + local row = store:_getRowRaw("h2") + assert.are.equal("finished", row.reading_status) + -- reading_status_updated_at must equal parse_modified_ms("2026-06-18") + assert.are.equal(expected_ts, row.reading_status_updated_at) + -- updated_at must have advanced past the pre-set 100ms value + assert.is_true(row.updated_at > 100, "updated_at should have advanced past 100") + store:close() + end) + + it("skips rows without a local file", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h3", title = "T", uploaded_at = 1, local_present = 0, + reading_status = "finished", reading_status_updated_at = 1 }) + local writes = {} + StatusSync.reconcileLocalStatuses(store, fake_deps({}, writes)) + assert.are.same({}, writes) + store:close() + end) + + it("first sync: pushes a bootstrap 'finished' to an opened KO book AND stamps the store", function() + -- Reported case 1: finished in Readest (never-stamped baseline), opened in + -- KOReader so its sidecar is the auto 'reading'. Must push complete down and + -- stamp the store so the book leaves bootstrap. + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h4", title = "T", file_path = "/b4.epub", local_present = 1, + reading_status = "finished", reading_status_updated_at = 0 }) + local summaries = { ["/b4.epub"] = { status = "reading", modified = "2026-01-01" } } + local writes = {} + StatusSync.reconcileLocalStatuses(store, fake_deps(summaries, writes)) + assert.are.equal("complete", writes["/b4.epub"]) -- pushed down, no downgrade + local row = store:_getRowRaw("h4") + assert.are.equal("finished", row.reading_status) + assert.are.equal(1750000000000, row.reading_status_updated_at) -- stamped with now_ms + store:close() + end) + + it("first sync: does not touch a book that is undefined in Readest and only 'reading' in KO (case 2)", function() + local store = LibraryStore.new({ user_id = "u1", db_path = ":memory:" }) + store:upsertBook({ hash = "h5", title = "T", file_path = "/b5.epub", local_present = 1, + reading_status = nil, reading_status_updated_at = 0 }) + local summaries = { ["/b5.epub"] = { status = "reading", modified = "2026-01-01" } } + local writes = {} + StatusSync.reconcileLocalStatuses(store, fake_deps(summaries, writes)) + assert.are.same({}, writes) + local row = store:_getRowRaw("h5") + assert.is_nil(row.reading_status) + store:close() + end) +end) diff --git a/apps/readest.koplugin/spec/library/syncbooks_spec.lua b/apps/readest.koplugin/spec/library/syncbooks_spec.lua index b4139b08..86d5ac01 100644 --- a/apps/readest.koplugin/spec/library/syncbooks_spec.lua +++ b/apps/readest.koplugin/spec/library/syncbooks_spec.lua @@ -213,6 +213,13 @@ describe("library.syncbooks", function() it("returns nil for nil input (defensive)", function() assert.is_nil(syncbooks._row_to_wire(nil)) end) + + it("emits readingStatusUpdatedAt in the wire payload", function() + local wire = syncbooks._row_to_wire({ hash = "h1", title = "T", + reading_status = "finished", reading_status_updated_at = 1750000000000 }) + assert.are.equal("finished", wire.readingStatus) + assert.are.equal(1750000000000, wire.readingStatusUpdatedAt) + end) end) -- ===================================================================== diff --git a/docker/volumes/db/init/schema.sql b/docker/volumes/db/init/schema.sql index f339e933..5b817b3c 100644 --- a/docker/volumes/db/init/schema.sql +++ b/docker/volumes/db/init/schema.sql @@ -17,6 +17,7 @@ CREATE TABLE public.books ( uploaded_at timestamp with time zone NULL, progress integer[] NULL, reading_status text NULL, + reading_status_updated_at timestamp with time zone NULL, group_id text NULL, group_name text NULL, metadata json NULL, diff --git a/docker/volumes/db/migrations/015_add_reading_status_updated_at.sql b/docker/volumes/db/migrations/015_add_reading_status_updated_at.sql new file mode 100644 index 00000000..f9ec1899 --- /dev/null +++ b/docker/volumes/db/migrations/015_add_reading_status_updated_at.sql @@ -0,0 +1,11 @@ +-- Migration 015: Add `reading_status_updated_at` to books +-- +-- Field-level last-writer-wins for reading_status. The books row carries +-- both reading_status (rare, intentional) and a denormalized progress +-- (every page turn) under one updated_at, so whole-row LWW lets progress +-- updates clobber a status change across devices (issue #4634). A dedicated +-- per-field timestamp lets the merge resolve reading_status independently. +-- Additive + nullable; NULL is treated as epoch 0 (oldest) by the merge. + +ALTER TABLE public.books + ADD COLUMN IF NOT EXISTS reading_status_updated_at timestamp with time zone NULL;