Skip to content

🐛 Use GitLab MR diff head SHA for CI statuses - #5176

Open
rylena wants to merge 2 commits into
ossf:mainfrom
rylena:fix-gitlab-mr-head-sha
Open

🐛 Use GitLab MR diff head SHA for CI statuses#5176
rylena wants to merge 2 commits into
ossf:mainfrom
rylena:fix-gitlab-mr-head-sha

Conversation

@rylena

@rylena rylena commented Aug 12, 2026

Copy link
Copy Markdown

Summary

  • query GitLab merge requests for diffHeadSha
  • use the diff head SHA as the associated merge request head SHA so CI status lookups target the MR head instead of the merge commit
  • avoid indexing empty merge commit SHAs when GitLab returns no merge commit

Fixes #3701

Verification

  • go test ./clients/gitlabrepo
  • git diff --check

Signed-off-by: Rylen Anil <rylen.anil@gmail.com>
@rylena
rylena requested a review from a team as a code owner August 12, 2026 05:08
@rylena
rylena requested review from jeffmendoza and spencerschrock and removed request for a team August 12, 2026 05:08
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Aug 12, 2026

@bilaldeveloper4312 bilaldeveloper4312 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 364afa82 against the behavior described in #3701.

Using diffHeadSha for AssociatedMergeRequest.HeadSHA matches the required semantics: CI-Tests needs the merge-request head as it existed before merge, while mergeCommitSha can be absent for fast-forward merges and identifies a different commit when present. Guarding the merge-commit lookup also prevents an empty SHA from becoming a map key. The regression test exercises the important distinction between the MR diff head and merge commit. I found no blocking issue in the changed code.

Focused verification passes locally with Go 1.25.6:

go test ./clients/gitlabrepo -run '^(TestZipUsesDiffHeadShaForMergeRequestHead|TestListRawCommits)$' -count=1

The full package run reaches the unchanged Windows-specific TestExtractTarball/Basic path-separator and file-lock failure, so I am not representing the entire package as passing in this environment.

Signed-off-by: Rylen Anil <rylen.anil@gmail.com>
@rylena

rylena commented Aug 14, 2026

Copy link
Copy Markdown
Author

Updated this branch with the latest upstream/main to clear the behind state. Verification after the merge: go test ./clients/gitlabrepo -run '^(TestZipUsesDiffHeadShaForMergeRequestHead|TestListRawCommits)$' -count=1 and git diff --check HEAD~1..HEAD.

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

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

BUG: CI-Tests: internal error: internal error: Client.Repositories.ListStatuses: error getting commit statuses

2 participants