Skip to content

fix(changeprovider): validate GitHub head on every page - #828

Open
Tmwakalasya wants to merge 4 commits into
uber:mainfrom
Tmwakalasya:tuntu/fix-github-pagination-staleness
Open

Tmwakalasya wants to merge 4 commits into
uber:mainfrom
Tmwakalasya:tuntu/fix-github-pagination-staleness

Conversation

@Tmwakalasya

Copy link
Copy Markdown
Contributor

Why?

If a PR's head changes between GraphQL file pages, the provider can return files from different revisions under the originally submitted SHA. It retains the first page's head and validates only after collecting every page, so a different head on a later page goes unnoticed.

What?

Move the existing staleness check into the pagination loop, before appending each page's files. Every page must match the SHA in the submitted change URI; a mismatch returns an error immediately without returning partial change information.

Add table-driven regression tests for stale heads on the first, middle, and final pages, including stopping before any subsequent page is requested. The existing successful-pagination test remains the control.

Test Plan

  • Confirmed the new regression cases fail before the fix: later-page mismatches return no error, and a stale first page still fetches the next page.
  • Passed the full provider target: ./tool/bazel test //submitqueue/extension/changeprovider/github:go_default_test --nofetch --test_output=errors.
  • Passed go vet -mod=readonly ./submitqueue/extension/changeprovider/github.
  • Passed make fmt lint check-tidy check-gazelle and git diff --cached --check.
  • Full repository and integration suites were not run; the change is scoped to the provider's pagination validation.

Issue

No linked issue.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant