Repository navigation
feat: Let API clients say when a position was read - #41
Conversation
A client that syncs some time after reading (an importer that runs every hour, say) had its progress stamped with the time of the request, so the reading timeline showed the read at the wrong time and an older position could count as the newest. PUT /api/v1/progress now takes an optional `timestamp` in unix seconds and stores that instead. A future time is capped at now; a missing one, or one before 2000, means now. /syncs/progress and the batch route keep using the time of the request, so a reader with an unset clock can't backdate its own progress.
|
Caution Review failedThe pull request is closed. Walkthrough
Merge Risk:
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PUT /api/v1/progressalways stored the time of the request. That's right for a reader, which syncs while you read. It's wrong for a client that syncs later, like a script that runs every hour and copies over where you got to. Something read at 7:10 then shows up in the reading timeline at whatever time the script ran, and "newest wins" treats it as newer than it is.This lets the client send
timestamp(unix seconds) with the position, and the server stores that instead. The progress row, the progress log behind the timeline, and auto-finish all use it. Change sequence numbers aren't affected, so incremental sync feeds keep working the same.Some limits so a bad clock can't do damage:
PUT /api/v1/progressreads it.PUT /syncs/progressand the batch route keep using the time of the request, since those are what readers use and a reader's clock isn't always set.Testing
test/progress-client-time.test.tscovers:Without the change in
kosync.tsthe test fails. The full suite passes (689 tests), and so doestsc --noEmit. The API docs are updated.