CXP-936; CXP-1010 fix pagination by persisting pageSize on token - #86
Conversation
…change ListTicketSchemas derived the /project/search page size from the caller's p.Size on every call, but its resume token only recorded a page-relative project index, not the size that index was computed against. C1's driver shrinks the requested page size as ListTicketSchemas accumulates toward its 100-schema cap, so a resume could refetch a smaller window at the same offset than the one the stashed index was valid for: the index landed past the end of the new window (dropping the rest of it), and the offset then advanced by the shrunk window length instead of the true position, causing part of the window to be re-emitted on the next call. Stash the window size (ProjectPageSize) alongside the resume index so a mid-window resume always refetches the identical window, regardless of what page size the caller sends on that call. A freshly started window (after the prior one is fully consumed) is unaffected and still sizes off the caller's current request. Fixes CXP-936. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Connector PR Review: CXP-936; CXP-1010 fix pagination by persisting pageSize on tokenBlocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0 Review Summary The new commit reworks the Security Issues None found. Correctness Issues None found. Suggestions
Prompt for AI agents |
| issueTypeIndex := tok.IssueTypeIndex | ||
| resumedProjectIndex := tok.ProjectIndexInPage | ||
|
|
||
| if projectIndex > 0 && projectIndex >= len(projects) { |
There was a problem hiding this comment.
🟡 Suggestion: The guard covers the out-of-bounds case, but not the shift case: if a project is removed from the front of the pinned window mid-sync, the refetched window is shorter yet ProjectIndexInPage stays in bounds and now points at a different project. That silently skips one project and, worse, applies the stashed tok.Statuses to the wrong project via the projectIndex == resumedProjectIndex branch (line 502). Pinning a project key/ID alongside the index and verifying it matches on resume (falling back to a scan or a window advance) would make the resume robust to any window mutation, not just shrinkage past the end.
Confidence: medium — requires a project to be deleted or become inaccessible mid-sync.
There was a problem hiding this comment.
This is going way out of scope. I created this ticket for this: CXP-1010
Must review again
Requires review
JavierCarnelli-ConductorOne
changed the title
A project deleted or made inaccessible mid-sync can shift the pinned project window without the stashed resume index going out of bounds, silently skipping a project and misapplying its cached statuses to whatever project now sits at that index. Stash the project key alongside the index and relocate by key on resume instead of trusting position alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Requires a review on the extended fix
The identity check only ran for projectIndex > 0, so a stashed resume at index 0 (cap hit mid-first-project) never verified the window still matched, letting stale statuses apply to a different project. Also switch the stashed identifier from project.Key (user-renamable) to project.ID (immutable) to avoid false mismatches on rename. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Requires revision
The relocation guard treated "no match found" as "nothing left to process" and always skipped past the whole window, even when other, untouched projects still sat at or after the stashed index. Only take that shortcut when the index is actually out of bounds; otherwise resume the window from the current index as if visiting it fresh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Requires a new review, once again
3 participants
ListTicketSchemas derived the /project/search page size from the caller's p.Size on every call, but its resume token only recorded a page-relative project index, not the size that index was computed against. C1's driver shrinks the requested page size as ListTicketSchemas accumulates toward its 100-schema cap, so a resume could refetch a smaller window at the same offset than the one the stashed index was valid for: the index landed past the end of the new window (dropping the rest of it), and the offset then advanced by the shrunk window length instead of the true position, causing part of the window to be re-emitted on the next call.
Stash the window size (ProjectPageSize) alongside the resume index so a mid-window resume always refetches the identical window, regardless of what page size the caller sends on that call. A freshly started window (after the prior one is fully consumed) is unaffected and still sizes off the caller's current request.
Fixes CXP-936.