Repository navigation
docs(sync): require the positive sync head on the first push - #249
Conversation
The sync tutorials told clients to send `lastMutationAt=-1` for a user's first mutation and to read `-1` as a "first-sync state". Both pinned specs say the opposite: the head returned by GET already includes virtual resources such as the default Favorites collection, so a new user has a positive head, and `-1` is rejected with a 409 OutOfSyncError. - describe `lastMutationAt` as the push precondition, separate from the `mutationsSince` pull cursor - rewrite the first-sync table, flowchart and steps around the positive head - document the page-1 continuation head that GET pagination requires - drop the retired "First sync detected..." error variant - add a test pinning both specs and both tutorials to this contract
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9316f18d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The guide says Sync is pre-live only, but the pre-live spec does not declare a `lastMutationAt` parameter on GET /v1/sync; only the production spec does. Scope the pagination rule accordingly: keep the same `mutationsSince` across pages, and use the page-1 continuation head where the deployment accepts it.
Production rejects GET /v1/sync page 2+ without the page-1 lastMutationAt, or with a different one, with 409 OutOfSyncError (verified live). Describe it as required rather than optional. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Production GET page 2+ requires the page-1 head; a changed head means restart the pull, a missing head is a request bug (no restart loop). - Pre-live does not accept lastMutationAt on GET: compare each page's returned head with page 1's instead. - Keep mutationsSince, resources and limit fixed across pages. - State the rule once in Step 2 and link to it; mention the GET continuation use in Key Concepts. - offline-first-patterns: push with the head returned by the pull, not the mutationsSince cursor. - handling-conflicts: add the mid-pull 409 case and use the pull cursor, not the head, as mutationsSince when recovering. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Resolve conflicts with quran#244 (Sync live in production): keep main's first-sync wording and cursor model, keep this PR's pagination continuation rules and mid-pull 409 case. Drop the 'not the pull cursor' note, which contradicted main, and update the PR's test to main's wording. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
The Sync tutorials told clients to push their first mutation with
lastMutationAt=-1and to read-1back fromGET /v1/syncas a "first-sync state". Both bundled specs say the opposite:POST /v1/sync→lastMutationAtdescription: "The effective sync head returned by the get mutations endpoint. It includes virtual resources such as the default Favorites collection, so even a new user must send the positive timestamp returned by GET rather than -1. A stale or different value will be rejected." (openAPI/user-related-apis/v1.json,openAPI/user-related-apis/pre-live/v1.json)GET /v1/sync→ responselastMutationAt: "Effective sync head, including virtual resources such as the default Favorites collection."lastMutationAtquery parameter (used for pagination) hasminimum: 0, and the 409 examples no longer contain theFirst sync detected. Please use lastMutationAt=-1 for initial sync.variant.A client that follows the current docs would get a
409 OutOfSyncErroron its very first push.Changes
docs/tutorials/sync/getting-started.mdxlastMutationAtas the push precondition (the server-assigned sync head) and state explicitly that the pull cursor ismutationsSince.GET /v1/syncpagination requires: page 2+ sends thelastMutationAtreturned by page 1, and a changed head returns409and restarts the pull.docs/tutorials/sync/handling-conflicts.mdx-1returns the ordinaryOutOfSyncError.tests/sync-docs-last-mutation-at.test.cjs(new)Testing
The full node test suite also runs in CI (
scripts/run-tests.cjs); the docusaurus-dependent tests needyarn install, which this change does not affect.Notes for reviewers
docs/user_related_apis_prelive/sync-local-mutations.api.mdxstill contains the old first-sync 409 example; it is regenerated byyarn gen-allat build time and was not hand-edited here.User Synccontract, which documents the same model (@quranjs/apiREADME: "lastMutationAt must be the exact current sync head").