## Problem The `ListCommits` RPC returns a raw commit SHA as the `PaginationCursor.next_cursor` value. For example, a client receives `b83d6e391c22777fca1ed3012fce84f633d7fed0` as the cursor and passes it back as `page_token` to fetch the next page. The `PaginationCursor.next_cursor` proto field is documented as "an opaque token provided to the caller to indicate what the caller should present as a page_token to get subsequent results." Returning a raw commit SHA violates this contract — clients can (and do) make assumptions about the cursor format, making it impossible to change the pagination strategy in the future without breaking them. Other paginated RPCs in Gitaly already use properly encoded cursors: - `GetTreeEntries` — base64-encoded JSON containing filename and tree OID - `ListPartitions` — base64-encoded JSON containing partition ID `ListCommits` should follow the same pattern. ## Affected clients in `gitlab-org/gitlab` ### `CommitCollectionWithNextCursor` [`lib/gitlab/gitaly_client/commit_collection_with_next_cursor.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/gitaly_client/commit_collection_with_next_cursor.rb) exposes `next_cursor` directly from the Gitaly response. The value is surfaced through `Repository#list_commits` to callers. After the Gitaly change, this value will be an encoded string instead of a raw SHA. ### `CommitsResolver` (GraphQL) [`app/graphql/resolvers/repositories/commits_resolver.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/resolvers/repositories/commits_resolver.rb) currently ignores the Gitaly cursor and constructs its own: ```ruby # FIX: use `response.next_cursor` instead of calculating commit manually end_cursor = Base64.encode64(commits.last.sha) if has_next_page ``` It then `Base64.decode64`s the cursor back to a raw SHA before passing it to Gitaly. This works today only because the cursor happens to be a raw SHA. After the Gitaly change, the resolver could use `response.next_cursor` directly instead of manually computing a cursor. ### Spec in `commit_service_spec.rb` [`spec/lib/gitlab/gitaly_client/commit_service_spec.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/lib/gitlab/gitaly_client/commit_service_spec.rb) asserts: ```ruby expect(response_1.next_cursor).to eq 'b83d6e391c22777fca1ed3012fce84f633d7fed0' ``` This hard-codes the assumption that the cursor is a raw SHA and will need updating. ## Proposal ### Gitaly 1. Encode the `NextCursor` as `base64(json({"commit_id": "<sha>"}))`, matching the pattern used by `GetTreeEntries` and `ListPartitions`. 2. When decoding `PageToken`, accept both the new encoded format and the legacy raw SHA format. If base64 or JSON decoding fails, treat the token as a raw commit SHA. This is the same backward-compatibility approach used by `GetTreeEntries.decodePageToken`. ### Rails 1. Update the spec assertion in `commit_service_spec.rb` to not assume the cursor is a raw SHA. 2. Optionally, update `CommitsResolver` to use `response.next_cursor` directly instead of computing its own cursor — removing the manual `Base64.encode64(commits.last.sha)` and `limit + 1` over-fetching pattern. ### Rolling deploy safety The Gitaly backward-compatible decoder ensures safety during rolling deploys: | Scenario | Behavior | |----------|----------| | New Gitaly emits encoded cursor → New Gitaly receives it | Decoded correctly ✅ | | Old Gitaly emits raw SHA → New Gitaly receives it | Fallback decode handles it ✅ | | New Gitaly emits encoded cursor → Old Gitaly receives it | Old server treats encoded string as OID, no match, returns from start ⚠️ (transient) | | Rails `CommitsResolver` passes raw SHA → Any Gitaly | Fallback decode handles it ✅ | The only degraded scenario is transient during the rolling deploy window and self-resolves once all Gitaly nodes are updated.